T11 code-quality reviewer flagged 1 IMPORTANT + 3 NITs (non-blocking, green-lit T11). Filed for batched cleanup before v0.7.x.
1. [IMPORTANT] Duplicate status state-machine logic
`apps/desktop/src/components/TranscriptionPill.tsx:38-56` (`statusLabel` + `isCancellable`) and `apps/desktop/src/components/TranscribeButton.tsx:127-142` (queue position label, percent calculation `total_ms !== null && total_ms > 0` → `Math.round((processed_ms/total_ms)*100)`) implement the same state-machine twice. Risk: drift between the two components.
Suggested fix: extract `apps/desktop/src/lib/transcriptionStatus.ts` exposing:
- `statusLabel(status: TranscriptionJobStatus): string`
- `isCancellable(status: TranscriptionJobStatus): boolean`
- `progressPercent(status: TranscriptionJobStatus): number | null`
Both components import + use; tests still exercise the components but the logic lives in one place.
2. [NIT] Missing exhaustive `never` default in statusLabel switch
`TranscriptionPill.tsx:38` — switch lacks the `const _exhaustive: never = ...` default. Other switches in the project (`StatusBar.errorKindLabel`, `lib/coreError.transcriptionErrorMessage`) use this pattern so adding a new variant becomes a compile-error.
3. [NIT] Missing aria-controls on pill button
`TranscriptionPill.tsx:174-205` — pill button has `aria-expanded` + `aria-haspopup` but no `aria-controls` linking to the popover (popover lacks an `id`). Add `id="transcription-popover"` + `aria-controls="transcription-popover"` for full ARIA contract.
4. [NIT] Missing WHY comment on Cancel discard
`TranscriptionPill.tsx:81-85` — Cancel handler discards the `ResultAsync` chain via `.mapErr(...)`. Same pattern as TranscribeButton.tsx:60-65 (also undocumented). Fire-and-forget intent matches the "event loop updates the slice on Cancelled" model, but a one-line WHY comment matching file's WHY-density would be in keeping.
Filed under ASR v2 milestone for batched cleanup.
T11 code-quality reviewer flagged 1 IMPORTANT + 3 NITs (non-blocking, green-lit T11). Filed for batched cleanup before v0.7.x.
1. [IMPORTANT] Duplicate status state-machine logic
`apps/desktop/src/components/TranscriptionPill.tsx:38-56` (`statusLabel` + `isCancellable`) and `apps/desktop/src/components/TranscribeButton.tsx:127-142` (queue position label, percent calculation `total_ms !== null && total_ms > 0` → `Math.round((processed_ms/total_ms)*100)`) implement the same state-machine twice. Risk: drift between the two components.
Suggested fix: extract `apps/desktop/src/lib/transcriptionStatus.ts` exposing:
Both components import + use; tests still exercise the components but the logic lives in one place.
2. [NIT] Missing exhaustive `never` default in statusLabel switch
`TranscriptionPill.tsx:38` — switch lacks the `const _exhaustive: never = ...` default. Other switches in the project (`StatusBar.errorKindLabel`, `lib/coreError.transcriptionErrorMessage`) use this pattern so adding a new variant becomes a compile-error.
3. [NIT] Missing aria-controls on pill button
`TranscriptionPill.tsx:174-205` — pill button has `aria-expanded` + `aria-haspopup` but no `aria-controls` linking to the popover (popover lacks an `id`). Add `id="transcription-popover"` + `aria-controls="transcription-popover"` for full ARIA contract.
4. [NIT] Missing WHY comment on Cancel discard
`TranscriptionPill.tsx:81-85` — Cancel handler discards the `ResultAsync` chain via `.mapErr(...)`. Same pattern as TranscribeButton.tsx:60-65 (also undocumented). Fire-and-forget intent matches the "event loop updates the slice on Cancelled" model, but a one-line WHY comment matching file's WHY-density would be in keeping.
Filed under ASR v2 milestone for batched cleanup.