Repository navigation
Show transcription progress in the CLI - #113
Conversation
wispr-cli went silent for the entire transcription of a long file. On a 50 minute recording it printed the model, the load time, the audio duration and a sample count, then nothing for several minutes — with no way to tell whether it was working or hung. --verbose did not help, because nothing between "decoded" and "done" reported anything. Three separate causes: - Neither WhisperService nor ParakeetService surfaced the progress that WhisperKit and FluidAudio already expose internally. - AudioFileDecoder.decode read the whole file in one pass with no tick. - In its default mode the CLI dups /dev/null over STDERR_FILENO for the whole run to silence FluidAudio's INFO logging, so anything written to stderr during transcription was discarded. Engines now report a position in the audio; the CLI owns all display policy. WisprCore gains a ProgressUpdate value type and a transcribe(_:language:onProgress:) overload added as a protocol requirement with a default implementation, so every existing call site and conformance compiles and behaves exactly as before. The GUI app is untouched. WhisperService reports from WhisperKit's segmentCallback (absolute segment timestamps give the authoritative audio position) with the per-token callback as a liveness signal only, returning nil so decoding is never interrupted. ParakeetService drains FluidAudio's transcriptionProgressStream. AudioFileDecoder takes a defaulted onProgress, throttled to a quarter second of audio. In the CLI, a ProgressReporter actor takes engine events through an AsyncStream with bufferingNewest(1) — yield is non-blocking and a burst of token-rate events collapses to the latest, so there is no queue and no backpressure on the decoder — and merges them with a 100 ms ticker. That split is what keeps the display animated during the ~30 s gaps between engine reports, so slow is distinguishable from hung. Output goes to the descriptor saved before the /dev/null redirection, and isatty is checked against that same descriptor. Adds --progress auto|always|never (also WISPR_PROGRESS) and --quiet. A TTY gets a single line redrawn in place with a bar, position, elapsed, ETA and realtime factor; a non-TTY gets periodic plain lines with no escape sequences; stdout stays a clean transcript pipe. Durations are now human-readable, so "22.556551125 seconds" reads "22.6s". Verified on a 37 minute file: the transcript is byte-identical to main's, so the reporting cannot have altered the result.
sebsto
left a comment
There was a problem hiding this comment.
Summary — excellent work, but it does not build under Xcode as-is; one blocking fix before merge.
This closes #112: wispr-cli now shows transcription progress — an in-place redrawn line on a TTY, throttled plain lines otherwise, gated by --progress auto|always|never / --quiet. The layering is the highlight: engines only report a position in the audio, and every display decision (throttle, ETA, TTY, narrowing) lives in the CLI, so nothing about presentation leaks into WisprCore.
What I checked hardest, all sound:
- Strictly additive and source-compatible. The new
transcribe(_:language:onProgress:)is a protocol requirement with a default implementation forwarding to the existing method, so every current conformer — includingMockTranscriptionEngine, which still implements only the two-arg form — compiles and behaves unchanged. - The transcript cannot have changed. Whisper's token
callbackreturnsnil(never interrupts decoding) andsegmentCallbackis read-only; Parakeet drains FluidAudio'stranscriptionProgressStreamin a pumpTaskcancelled ondefer. The byte-identical-output verification backs this up. - No backpressure on the decoder. Synchronous
@Sendableengine callbacksyieldinto anAsyncStreamwith.bufferingNewest(1), collapsing token-rate bursts to the latest — no unbounded queue, noawaitfrom the callback. - The stderr trap is handled. Progress writes to the descriptor saved before
/dev/nullis duped overSTDERR_FILENO, andisattyis checked against that same fd. The^Ccursor restore uses aDispatchSourceSignal(not a raw C signal handler) thenSIG_DFL+ re-raise — the correct pattern.
One blocking finding (build), one house-style concurrency flag, one Low. The Whisper path is implemented against WhisperKit's documented callbacks but the author discloses it was not run end-to-end (only parakeet-v3 installed) — worth a human run with a Whisper model before release, but not a code issue.
swift build passed but xcodebuild failed with "cannot find type 'TranscriptionProgressHandler' in scope". WisprCore and WisprCLI use explicit file lists in project.pbxproj — only the test targets are PBXFileSystemSynchronizedRootGroups — so the three new files were invisible to the Xcode build and the types they declare did not exist. Adds the same four entries per file that #108 used for FileAccess.swift: a PBXBuildFile, a PBXFileReference, a group child, and a Sources build phase member. ProgressUpdate.swift goes to WisprCore, DurationFormat.swift and ProgressReporter.swift to WisprCLI. wisprTests needs nothing, being a synchronized group. Verified with CI's exact invocation: BUILD SUCCEEDED.
There was a problem hiding this comment.
🟡 Changes recommended
Critical project-registration and compile issues remain, along with unresolved behavioral issues requiring fixes.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds end-to-end transcription progress reporting to wispr-cli, including engine callbacks, terminal/plain rendering, new flags, and readable timing output.
Changes:
- Adds progress APIs and forwarding across WisprCore engines and decoding.
- Adds CLI progress rendering,
--progress,WISPR_PROGRESS,--quiet, and duration formatting. - Adds progress tests and feature documentation.
File summaries
| File | Summary and final review comments |
|---|---|
wisprTests/TranscriptionProgressTests.swift |
Adds progress plumbing tests. No final comments. |
Sources/WisprCore/Services/WhisperService.swift |
Adds Whisper progress reporting. moderate (1 vote): callback mapping lacks automated coverage and was not tested end-to-end. |
Sources/WisprCore/Services/TranscriptionEngine.swift |
Adds the additive progress API. No final comments. |
Sources/WisprCore/Services/ProgressUpdate.swift |
Defines progress types. critical (3 votes): missing Xcode project registration prevents these types from being included in project builds. |
Sources/WisprCore/Services/ParakeetService.swift |
Forwards Parakeet progress. moderate (1 vote): the supported EOU model bypasses progress reporting. |
Sources/WisprCore/Services/CompositeTranscriptionEngine.swift |
Forwards progress to the active engine. No final comments. |
Sources/WisprCore/Services/AudioFileDecoder.swift |
Adds decode progress callbacks. No final comments. |
Sources/WisprCLI/WisprCLI.swift |
Wires CLI options and orchestration. critical (1 vote): an internal ProgressPreference is exposed through an invalid public member. moderate (2 votes): --quiet --verbose still emits diagnostics. moderate (2 votes): untagged decode events can corrupt transcription-phase progress. nit (2 votes): silent mode still passes a non-nil handler. nit (1 vote): verbose non-TTY behavior conflicts with the documented auto mode. |
Sources/WisprCLI/ProgressReporter.swift |
Implements rendering and lifecycle. critical (3 votes): missing Xcode target registration. moderate (2 votes): lacks focused renderer and formatting tests. moderate (1 vote): plain output can remain silent for up to five seconds. moderate (1 vote): cancellation can enqueue stale frames after finishing. nit (1 vote): ProgressStyle lacks Equatable. nit (1 vote): ProgressPreference lacks Equatable. |
Sources/WisprCLI/DurationFormat.swift |
Adds duration formatting. critical (1 vote): missing Xcode target registration. nit (1 vote): formatting boundary cases lack automated tests. |
.kiro/specs/cli-transcription-progress/tasks.md |
Documents implementation tasks. No final comments. |
.kiro/specs/cli-transcription-progress/requirements.md |
Documents requirements. No final comments. |
.kiro/specs/cli-transcription-progress/design.md |
Documents the design. No final comments. |
Review details
Suppressed comments (8)
Sources/WisprCLI/DurationFormat.swift:21
- The new duration-formatting functions have no automated tests, while the existing Swift suite tests analogous duration boundaries (for example,
wisprTests/TranscriptStoreTests.swift:290-294). Add focused cases for sub-minute rounding, 60/3600-second transitions, invalid values, and clock formatting so changes to terminal output are caught.
nonisolated func formatDuration(_ seconds: Double) -> String {
Sources/WisprCLI/ProgressReporter.swift:50
ProgressStyleis compared with==and!=in this file and fromWisprCLI, but it does not conform toEquatable. The CLI target therefore fails to compile; add the conformance.
nonisolated enum ProgressStyle: Sendable {
Sources/WisprCLI/ProgressReporter.swift:60
ProgressPreferenceis compared with==inWisprCLI.swift, but it does not conform toEquatable. This also prevents the CLI target from compiling.
nonisolated enum ProgressPreference: String, Sendable, CaseIterable {
Sources/WisprCLI/ProgressReporter.swift:172
- When plain output is enabled for a non-TTY, this five-second interval allows progress to go silent for up to five seconds whenever the fraction changes by less than 5%. The 100 ms ticker only calls
renderPlain; it does not bypass this guard, so--verboseand--progress alwaysdo not meet the required once-per-second reporting cadence. Use a one-second-or-shorter maximum interval (or otherwise enforce that bound).
private static let plainMinInterval = Duration.seconds(5)
Sources/WisprCLI/ProgressReporter.swift:284
- Cancelling these tasks is fire-and-forget. A render task that has already passed its cancellation check can still enqueue
tick()afterfinish(); because the phase state is left populated, that tick may redraw after the cursor is shown and interleave a stale progress frame with the transcript or error output. Add a finished/generation guard and wait for already-queued render work to quiesce before returning.
intakeTask?.cancel()
renderTask?.cancel()
intakeTask = nil
renderTask = nil
eventSink.finish()
Sources/WisprCLI/WisprCLI.swift:129
- The option help says
autois enabled only when stderr is a terminal, butprogressStylereturns.plainLinesfor.autowhenverboseis true and stderr is non-TTY (line 305). Therefore--verbose 2>fileemits progress despite the documented rule. Clarify the--verboseexception in the help or change the style logic so the public contract is consistent.
Progress indicator: auto (only when stderr is a terminal), always, \
or never. Also settable via WISPR_PROGRESS.
Sources/WisprCore/Services/ParakeetService.swift:493
- This early return is reachable from the CLI:
discoverDownloadedModels()exposesparakeet-eou-160ms, and--modelaccepts that name. It bypassesonProgressentirely, so a supported EOU CLI transcription stays indeterminate for its whole run despite the progress contract covering Parakeet models; the comment that the CLI never uses EOU is not true. Either report position while processing EOU audio or explicitly disallow this model in the CLI.
if activeModelName == ModelInfo.KnownID.parakeetEou {
return try await transcribeWithEou(audioSamples)
Sources/WisprCore/Services/WhisperService.swift:470
- The new Whisper callback path has no automated coverage: the existing WhisperService tests stop at missing-model/error paths, while the added progress tests use fake engines and never exercise WhisperKit's callback-to-position mapping. Since this integration was also not run end-to-end per the PR description, a callback-level test seam or an integration test is needed to catch a wrong callback contract or timestamp/window mapping while still allowing model-free CI.
let tokenCallback: TranscriptionCallback = onProgress.map { report in
{ progress in
// Coarse lower bound: the start of the window being decoded.
// Consumers clamp monotonically, so this never walks the
// displayed position backwards after a segment report.
- Files reviewed: 14/14 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved compile, progress lifecycle, output-suppression, throttling, and model-coverage issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (9)
Sources/WisprCLI/ProgressReporter.swift:60
progressStylecomparesProgressPreferencewith.never, but the enum has noEquatableconformance, so this new comparison also fails to compile. AddEquatableto the enum declaration.
nonisolated enum ProgressPreference: String, Sendable, CaseIterable {
Sources/WisprCLI/ProgressReporter.swift:281
- If an error occurs while a phase is active, cancelling
renderTaskdoes not retract a tick that is already queued on the actor. Becausefinish()leavesphaseandphaseStartset, that tick can redraw after the cursor is shown and after transcript/error output begins. Clear the phase state before cancelling the tasks, or add a finished guard checked bytick().
func finish() {
intakeTask?.cancel()
renderTask?.cancel()
Sources/WisprCLI/ProgressReporter.swift:223
- The stream intake is independent of phase lifecycle calls:
yieldonly enqueues an event, soendPhase()can run before the final decoder/engine update is applied. That queued update may then be applied after the nextbegin, carrying the previous phase's position and speed into the new phase. Add a phase-generation tag or an awaited drain/barrier before transitioning phases.
for await update in stream {
await self?.apply(update)
}
Sources/WisprCLI/WisprCLI.swift:196
--quietdoes not suppress the SDK's stderr when it is combined with--verbose: this branch skipssuppressStderr(), andlogsEnabledonly silencesProgressReporter, not FluidAudio/Whisper logging. Since quiet is documented to suppress all stderr output, make stderr suppression also apply wheneverconfig.quietis true (while still restoring it before reporting errors).
let savedFd = config.verbose ? Int32(-1) : suppressStderr()
defer { if !config.verbose { restoreStderr(savedFd) } }
Sources/WisprCLI/WisprCLI.swift:283
- This always passes a non-nil handler, even when
styleis.silent(--quiet,--progress never, or default non-TTY). Whisper therefore still constructs a progress update and performs anAsyncStream.yieldfor every decoded token with no consumer, adding avoidable token-rate overhead in the modes that disabled progress. Passnilto the engine when reporting is disabled, or make these handler accessors return an optional that is nil for the silent style.
let result = try await engine.transcribe(
samples,
language: language,
onProgress: reporter.transcriptionHandler()
)
Sources/WisprCLI/WisprCLI.swift:135
- The new
--quietflag is documented to suppress all stderr status output, but the pre-run diagnostic above still printsModels rootwhenever--verboseis combined with--quiet. Gate that diagnostic on!quietas well so this mode is actually silent.
@Flag(name: [.long, .short], help: "Suppress all progress and status output on stderr.")
var quiet = false
Sources/WisprCLI/WisprCLI.swift:299
- The added tests cover only
ProgressUpdateand engine forwarding; none exerciseprogressStyleorProgressReporter, which contain the new TTY/plain/silent mode behavior. Add focused CLI tests for these mode boundaries so regressions in--quiet, redirected stderr, and redraw/plain-line formatting are not left to manual verification.
static func progressStyle(
preference: ProgressPreference,
quiet: Bool,
verbose: Bool,
output: ProgressOutput
Sources/WisprCLI/WisprCLI.swift:282
- The intake task consumes
eventsindependently of this phase-transition call, so the final decode callback can still be buffered whendecodereturns.begin(.transcribing, ...)resets the reporter, but does not drain or tag queued events; a stale decode update can then be applied to transcription, making it start at 100% and suppressing the real engine positions. Synchronize the event stream at phase boundaries or associate each update with its phase before resetting state.
await reporter.begin(.transcribing, total: audioSeconds)
let result = try await engine.transcribe(
samples,
language: language,
onProgress: reporter.transcriptionHandler()
Sources/WisprCore/Services/ParakeetService.swift:494
- The CLI can reach this branch:
ParakeetService.availableModels()exposesparakeet-eou-160ms, andWisprCLIaccepts/discovers that model. Returning throughtranscribeWithEouwithout invokingonProgresstherefore leaves CLI progress indeterminate for a supported model, contrary to the determinate progress path. Either report progress for EOU transcription or exclude/reject this model in the CLI.
if activeModelName == ModelInfo.KnownID.parakeetEou {
return try await transcribeWithEou(audioSamples)
}
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Lite
Fixes a real correctness bug, closes the test gap, and tightens --quiet. Stale-event race: event intake is asynchronous, so the decoder's final report — a position equal to the whole audio length — could be applied after begin(.transcribing) had reset the state. That pinned the bar at 100%, and the monotonic clamp then rejected every real engine update for the rest of the run, silently defeating the feature. Events now carry the generation of the phase that produced them and stale ones are dropped. No per-token cost when progress is off: the handler factories return nil for the silent style, so the engines build nil backend callbacks and WhisperKit does no per-token work. A no-op closure would still have cost a call per decoded token. --quiet is now authoritative for stderr. It gates the "Models root:" diagnostic, and — found while testing that — --verbose had also disabled the SDK-log suppression, so `--quiet --verbose` printed 20 lines of FluidAudio INFO logging. A flag promising to suppress all stderr output has to beat one that merely asks for more of it. Replaces the nonisolated(unsafe) signal-source storage with a Mutex. The write-once discipline made the unchecked version sound, but this costs nothing and keeps the file free of concurrency escape hatches. Adds 33 tests for the CLI layer, previously covered only by manual terminal captures: duration and clock formatting boundaries, style resolution from the flags, preference parsing, SDK-log suppression precedence, the reporter state machine (monotonic clamping, clamping to total, phase reset, generation tagging) and bar geometry. This needed WisprCLI on the SPM test target; the Xcode scheme's Build action contains only WisprApp, so build-xcode is unaffected. Renames ProgressReporter to TerminalProgressReporter: Foundation ships its own ProgressReporter in the macOS 26 SDK, which is unambiguous inside the module but not from the test target. Same collision-avoidance already applied to ProgressUpdate and TranscriptionProgressHandler. Also corrects a wrong value repeated in the design table and in a test: formatDuration(0.85) is "0.8s", not "0.9s", because 0.85's nearest Double is below 0.85. Transcript remains byte-identical to main across 13 runs including --verbose --progress always with the pump task active.
Review round complete — 13 threads, all replied and resolvedTwo commits: Implemented — 9
Dismissed as factually incorrect — 3. Each rebutted with a standalone repro in its thread rather than a bare assertion: empty Partially implemented — 1. The 5 s non-TTY throttle vs the once-per-second requirement was a genuine spec/code inconsistency, but I fixed the spec: the once-per-second guarantee belongs to the interactive display (100 ms ticker), while the non-TTY channel is a log stream where one line per second means ~3 000 lines for a 50-minute file. R1.1 now scopes it and R1.1a states the log cadence. Happy to tighten the constant if you disagree. The one that matteredThe race would have silently defeated the entire feature. Second
|
| invocation | stderr |
|---|---|
--quiet |
0 bytes |
--quiet --verbose |
0 bytes (was 2 863) |
--verbose |
SDK logs + phase lines + Models root |
| default | Using model: parakeet-v3 |
Also worth flagging
ProgressReporter is now TerminalProgressReporter. Foundation ships its own ProgressReporter in the macOS 26 SDK — fine inside the module, ambiguous once the test target imported WisprCLI. Same collision-avoidance already applied to ProgressUpdate and TranscriptionProgressHandler.
The tests also caught a wrong value I'd written into the design doc: formatDuration(0.85) is 0.8s, not 0.9s, since 0.85's nearest Double is below 0.85. Corrected in the doc and the test.
Determinism
One run this round differed from main by a single word while the 737-test suite was executing concurrently. It did not reproduce. 13 subsequent controlled runs — idle, 4-way concurrent, during the full test suite, and with progress fully enabled (--verbose --progress always, pump task active) — are byte-identical to main on both branches (md5 5fb684d7…, 40 830 bytes). I could not attribute it to this change and could not reproduce it; flagging it rather than leaving it unsaid.
Still true from the original PR: only parakeet-v3 is downloaded here, so the Whisper progress path is compile-verified but not exercised end-to-end.
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved moderate findings affect terminal rendering and supported-model progress forwarding, while the Whisper callback path lacks direct coverage.
Review details
Suppressed comments (7)
Previously missed (2) — in code that hasn't changed since the last review.
.kiro/specs/cli-transcription-progress/requirements.md:96
- This requirement is inconsistent with the implemented and documented CLI contract:
progressStyleemits plain lines for non-TTY output when--progress alwaysis set, not only for--verbose. Please include--progress alwayshere so the requirements do not describe a mode the CLI intentionally supports as silent.
Sources/WisprCore/Services/WhisperService.swift:489 - The backend-specific progress path is not covered by the repository's WhisperService tests: those tests stop before a model is loaded, while the new progress tests use synthetic engines, so neither the segment position mapping nor the nil-returning token callback is exercised. Please add an injectable/fake WhisperKit seam or an integration fixture that invokes this overload and asserts the forwarded updates; otherwise this path can regress while the suite remains green.
.kiro/specs/cli-transcription-progress/design.md:248
- This rendering section still says non-TTY plain lines are
--verbose-only and marks every other non-TTY run silent, but the CLI also intentionally supports--progress always(and the PR description advertises that mode). Update both the Plain lines and Silent descriptions to match the actual option behavior.
**Plain lines (non-TTY, `--verbose`).** No `\r`, no escapes. Emitted on a 5 s
interval or a 5 % delta, whichever comes first — a log cadence, deliberately not
the interactive channel's 10 Hz refresh (see R1.1a):
[progress] transcribing 38% 19:23/50:53 elapsed 2:41 eta 4:22 11.8x
**Silent.** `--quiet`, `--progress never`, or non-TTY without `--verbose`.
.kiro/specs/cli-transcription-progress/tasks.md:28
- This checklist still points to
Sources/WisprCLI/ProgressReporter.swift, but the implementation was renamed toTerminalProgressReporter.swiftand the later review note documents that rename. Leaving the old path here sends future contributors to a file that does not exist.
- [x] 2.2 New `Sources/WisprCLI/ProgressReporter.swift`: phases, actor state,
Sources/WisprCLI/TerminalProgressReporter.swift:324
- Cancelling the render task does not remove a tick that is already queued at
await self?.tick(). On an error pathphaseandphaseStartremain set, so that tick can redraw the stale progress line afterfinish()shows the cursor and interleave it with the error output. Clear the phase state (or add a finished guard) before returning fromfinish().
eventSink.finish()
eraseLine()
Sources/WisprCLI/WisprCLI.swift:205
- In verbose TTY runs,
savedFdis-1, so FluidAudio's INFO writes and the reporter's redraws share the same terminal descriptor without coordination. An SDK log can arrive while the progress line is active; it appends a newline before the next\rredraw, leaving a corrupted progress frame on the previous row instead of the advertised single-line display. Suppress/redirect SDK logs while interactive progress is enabled, or route verbose logs through a coordinated output path.
let suppressingSDKLogs = Self.shouldSuppressSDKLogs(
verbose: config.verbose,
quiet: config.quiet
)
let savedFd = suppressingSDKLogs ? suppressStderr() : Int32(-1)
Sources/WisprCore/Services/ParakeetService.swift:494
- This branch is reachable from the CLI:
WisprCLI.discoverDownloadedModelsincludesparakeet-eou-160ms, and model resolution can select it. Returning throughtranscribeWithEoudropsonProgress, so that supported CLI model shows no position or determinate progress at all despite the progress contract. Either forward progress through the EOU path or explicitly exclude this model from CLI selection and document the limitation.
if activeModelName == ModelInfo.KnownID.parakeetEou {
return try await transcribeWithEou(audioSamples)
}
- Files reviewed: 16/17 changed files
- Comments generated: 1
- Review effort level: Lite
With tiny downloaded, the WhisperKit callback path is now verified end-to-end rather than only compile-checked: positions advance in exact 30 s increments from segmentCallback's absolute segment timestamps, and the interactive line renders in full at 110 columns. Establishing transcript identity needed care. tiny is non-deterministic on repetitive synthetic TTS -- on main, independently of this change: five main runs on one such file gave five different transcripts spanning 1334-3678 bytes. On natural speech main is exactly reproducible, and against that baseline every branch configuration (silent, progress enabled, verbose+progress) matches byte for byte on a 504 s file where the callbacks fire ~17 times.
Whisper path now verified end-to-endYou downloading Positions advance in exact 30 s increments, which confirms Interactive line at a 110-column pty, full width, nothing truncated, reaching 100%:
R6.2 — and a false alarm worth reportingMy first comparison on the Whisper path looked like a regression: On natural, non-repetitive speech
Against those, every branch configuration matches byte for byte:
So the callbacks don't alter the transcript, on a file long enough for Recorded in the spec in 59d6f29. One incidental note: |
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved moderate findings remain in progress cleanup, duration formatting, input validation, and documentation.
Review details
Suppressed comments (7)
Previously missed (2) — in code that hasn't changed since the last review.
.kiro/specs/cli-transcription-progress/tasks.md:30
- This checklist still names
Sources/WisprCLI/ProgressReporter.swift, but the implementation and Xcode registration areTerminalProgressReporter.swift. The completed task therefore points at a file that does not exist; update the checklist (and the corresponding design section) to the actual implementation name.
Sources/WisprCore/Services/WhisperService.swift:470 - The new WhisperKit callback wiring is not covered by an automated success-path test: the added tests exercise fake
TranscriptionEngineconformers, while the existingWhisperServiceTestsonly reach the no-model guards. BecausesegmentCallback/callbackdetermine whether real positions are emitted and the callback must returnnilwithout changing the transcript, add a fixture or injectable backend test that exercises this overload with a loaded Whisper model.
.kiro/specs/cli-transcription-progress/design.md:21
- This design still refers to
ProgressReporterandProgressReporter.swift, while the shipped implementation isTerminalProgressReporterinSources/WisprCLI/TerminalProgressReporter.swift(the rename was needed to avoid the SDK collision). The diagram and API section now direct readers to a nonexistent type/file; update these references to match the implementation.
┌─ WisprCLI ────────────────────────────────────────────────────────┐
│ ProgressReporter (actor) │
│ ├── AsyncStream<Event>, .bufferingNewest(1) ← engine events │
│ ├── 100 ms ticker ← liveness │
│ ├── state: phase, position, total, phase start, run start │
│ └── TerminalRenderer (TTY single-line │ plain lines │ silent) │
.kiro/specs/cli-transcription-progress/requirements.md:96
- The requirements say non-TTY progress is emitted only with
--verbose, but the implemented and tested behavior also emits plain lines for the explicit--progress alwaysoption (WisprCLI.swift:322-326). Sincealwaysis a public mode and the CLI help promises it, this requirement is now contradictory; document the explicit opt-in as an allowed case.
4.4 When stderr is not a TTY (piped, redirected, CI), the CLI SHALL NOT emit
ANSI escapes or carriage returns. It SHALL emit periodic plain one-line updates
instead, and only when `--verbose` is set.
Sources/WisprCLI/DurationFormat.swift:28
- When
secondsis just below 60 (for example, 59.96), this branch is selected before rounding but%.1frounds the value to60.0s. That produces an invalid boundary format and contradicts the 60-second case (1m 0s); choose the unit after rounding so the display never emits60.0s.
if seconds < 60 {
return String(format: "%.1fs", seconds)
}
let total = Int(seconds.rounded())
Sources/WisprCLI/TerminalProgressReporter.swift:323
- Cancellation here is not a join: the render loop checks
Task.isCancelledbefore the separateawait self?.tick()hop, so it can pass that check, be cancelled while suspended, and enqueue a tick afterfinish()has erased the line and shown the cursor. On an error path the phase is still set, so that late tick can redraw progress after the CLI reports the error. Track a finished state thattick/render honor, or await the render task's completion before returning fromfinish().
intakeTask?.cancel()
renderTask?.cancel()
intakeTask = nil
renderTask = nil
eventSink.finish()
Sources/WisprCore/Services/ProgressUpdate.swift:44
fractionis documented to return eithernilor a value in0...1, but a caller or backend can constructProgressUpdate(processedSeconds: .nan, totalSeconds: 1)and themin(max(...))expression returnsNaN. Treat non-finite positions (and totals) as invalid before calculating the ratio so the public value type preserves its stated invariant.
guard let totalSeconds, totalSeconds > 0 else { return nil }
return min(max(processedSeconds / totalSeconds, 0), 1)
- Files reviewed: 16/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
The design still named Sources/WisprCLI/ProgressReporter.swift and the ProgressReporter actor, so a reader following it would look for a file and type that no longer exist. Updates the architecture diagram, the new-file heading, the actor sketch, the verification reference, and task 2.2. The one remaining mention of a bare ProgressReporter is deliberate: it describes Foundation's own type, which is why ours was renamed.
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved moderate findings affect duration formatting, reporter shutdown safety, and quiet-mode error suppression; Whisper callback coverage is also incomplete.
Review details
Suppressed comments (9)
Previously missed (1) — in code that hasn't changed since the last review.
Sources/WisprCLI/TerminalProgressReporter.swift:324
- Cancelling
renderTaskis cooperative: a tick that has already passed its cancellation check can still be queued on this actor and run afterfinish()returns. Becausephaseremains set, that tick can redraw afterwriteOutputor after the cursor is shown, corrupting the transcript/prompt. Clear the phase state (or add a finished guard) duringfinish()after erasing the current line.
.kiro/specs/cli-transcription-progress/design.md:242
- The design repeats the same mismatch: it labels plain output as non-TTY/
--verboseand lists every non-TTY/no-verbose run as silent, although an explicit--progress alwayscorrectly selects.plainLineseven without--verbose. Update this rendering section to document that explicit exception, matching the CLI and the requirements.
**Plain lines (non-TTY, `--verbose`).** No `\r`, no escapes. Emitted on a 5 s
interval or a 5 % delta, whichever comes first — a log cadence, deliberately not
the interactive channel's 10 Hz refresh (see R1.1a):
.kiro/specs/cli-transcription-progress/design.md:248
- This silent-mode list also omits the explicit non-TTY
--progress alwaysexception implemented byprogressStyle, so the design's mode matrix is internally contradictory. Include--progress alwaysin the non-silent cases.
**Silent.** `--quiet`, `--progress never`, or non-TTY without `--verbose`.
.kiro/specs/cli-transcription-progress/requirements.md:96
- This requirement says redirected/non-TTY progress is emitted only with
--verbose, but the implementation intentionally emits plain lines for an explicit--progress alwaysas well (progressStylehandles.always). Sincealwaysis the documented way to request progress in scripted runs, update the requirement (and matching design text) to include that explicit preference so the acceptance criteria do not contradict the CLI.
4.4 When stderr is not a TTY (piped, redirected, CI), the CLI SHALL NOT emit
ANSI escapes or carriage returns. It SHALL emit periodic plain one-line updates
instead, and only when `--verbose` is set.
Sources/WisprCLI/DurationFormat.swift:25
- For values just below one minute, this branch rounds to one decimal before deciding on units, so
formatDuration(59.96)returns60.0swhileformatDuration(60)returns1m 0s. Promote after rounding (or base the branch on rounded tenths) so the formatter does not emit a misleading 60-second sub-minute value.
if seconds < 60 {
return String(format: "%.1fs", seconds)
Sources/WisprCLI/WisprCLI.swift:187
- Because the
--quietredirection is installed only after the file existence check andresolveModel, failures in either preflight step escape the quiet path and are printed by ArgumentParser to fd 2. That contradicts R5.2 and the new help/comment promise of suppressing all stderr; start the saved-fd redirection before these checks, or explicitly define errors as an exception and update the requirement/help.
// Resolve the model before any stderr redirection, so a failure here is
// reported normally.
let modelName = try resolveModel(config.modelName)
Sources/WisprCLI/WisprCLI.swift:130
- The help text says
autoemits progress only when stderr is a terminal, butprogressStylereturns.plainLinesforautowhenverboseis true on a non-TTY. Thus--verbose 2>fileemits progress underauto; update the help text or remove the override so the documented flag behavior matches the implementation.
Progress indicator: auto (only when stderr is a terminal), always, \
or never. Also settable via WISPR_PROGRESS.
"""
Sources/WisprCore/Services/ParakeetService.swift:486
- This statement is not true for the CLI:
resolveModeldiscoversparakeet-eou-160msand--modelcan select it, sotranscribeWithEouis reachable from this command. That path currently emits no progress; document it as an intentional indeterminate fallback (or add/reject support) rather than claiming the CLI never uses it.
/// The EOU model takes the realtime path (`transcribeWithEou`), which the CLI
/// never uses, so it reports nothing.
Sources/WisprCore/Services/WhisperService.swift:470
- The new
tokenCallback/segmentCallbackwiring is only reached after a loaded WhisperKit model, but the existing WhisperServiceTests stop at the no-model error path and the PR explicitly leaves this path unverified. Add a deterministic backend seam or a model-backed test that invokes both callbacks (including the nil return), otherwise a callback API mismatch can ship unnoticed.
let tokenCallback: TranscriptionCallback = onProgress.map { report in
{ progress in
// Coarse lower bound: the start of the window being decoded.
// Consumers clamp monotonically, so this never walks the
// displayed position backwards after a segment report.
- Files reviewed: 16/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
Closes #112
What
wispr-clinow reports progress while it transcribes. On a TTY it draws one line, redrawn in place:Non-TTY runs get periodic plain lines with no escape sequences, and only when asked for:
New flags:
--progress auto|always|never(alsoWISPR_PROGRESS) and--quiet. Default isauto— on when stderr is a terminal.How
Engines report a position in the audio; the CLI owns all display policy. Nothing about presentation leaks into
WisprCore.WisprCore, strictly additive. AProgressUpdatevalue type and atranscribe(_:language:onProgress:)overload added as a protocol requirement with a default implementation that forwards to the existing method. Every existing call site and conformance compiles and behaves as before — includingMockTranscriptionEnginein the tests, which still implements only the two-argument version.AudioFileDecoder.decodegains a defaultedonProgress, so its call sites are source-compatible.I considered having the CLI call WhisperKit and FluidAudio directly to avoid touching the shared module, but that duplicates model-path resolution, hallucination filtering and error mapping into a second path that will drift. The rejected alternatives are recorded in the spec.
Whisper's position comes from
segmentCallback—TranscriptionSegment.endis an absolute timestamp, somax(end)is the authoritative audio position (~102 reports for a 50 minute file). The per-tokencallbackis a liveness signal only and returnsnil, so decoding is never interrupted. Parakeet drains FluidAudio'stranscriptionProgressStream.ProgressReporteractor. Engine callbacks are synchronous and@Sendable, so they cannotawaitinto an actor, and aTaskper token would be wasteful. They yield into anAsyncStreamwith.bufferingNewest(1):yieldis non-blocking and a burst of token-rate events collapses to the latest, so there is no unbounded queue and no backpressure on the decoder. One render task merges that stream with a 100 ms ticker. That split is what keeps the spinner moving during the ~30 s of audio between engine reports, so slow is distinguishable from hung.The stderr trap. In default mode the CLI dups
/dev/nulloverSTDERR_FILENOfor the whole run to silence FluidAudio's INFO logging, so progress written toFileHandle.standardErrorwould be discarded. Progress goes to the descriptor saved before the redirection, andisattyis checked against that same descriptor rather thanSTDERR_FILENO.Also replaces the raw
Durationinterpolation:22.556551125 secondsnow reads22.6s, andDecoded 48851727 samplesreadsAudio decoded in 12.4s (37:41 of audio).Verification
main's (40 830 bytes both sides), so the reporting cannot have altered the result.ProgressUpdateclamping, the protocol default implementation carrying a non-reporting engine, and composite forwarding to the engine owning the active model.2>file,--quiet(verified byte-empty stderr),--progress never,--verbose,WISPR_PROGRESS=always,--list-models.git diff --name-only maincontains nothing underSources/WisprApp/.Bugs found while verifying
nil, and the delta rule required a previous non-nilfraction, so percentages were suppressed until the 5 s timer. A phase finishing inside 5 s reported 0% and nothing else.Using model:was initially gated on the progress style, which silently dropped it in non-TTY runs. Status lines and the progress animation are now controlled separately; only--quietsilences both.Not covered
The Whisper path is implemented against WhisperKit's documented callbacks but was not exercised end-to-end: only
parakeet-v3is downloaded on my machine, so the Parakeet path is the one verified for real. Worth a second pair of eyes, or a run by someone with a Whisper model installed.