Unify local and remote transcription runs - #37
Conversation
|
Warning Review limit reached
More reviews will be available in 42 minutes and 30 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideIntroduce a unified transcription run module that drives both local-file and remote-URL ASR transcriptions with shared state/log semantics, split ASR/LLM runtime settings normalization, and refactor hooks and tests to use the new abstractions. Sequence diagram for unified local/remote transcription runsequenceDiagram
actor User
participant useTranscriptionFlow
participant runTranscription
participant fetchRemoteAudioForAsr
participant runAsrTranscription
User->>useTranscriptionFlow: transcribe(file) / importFromUrl(url)
useTranscriptionFlow->>runTranscription: runTranscription(input, settings, callbacks, options)
alt input.source == remote
runTranscription->>fetchRemoteAudioForAsr: fetchRemoteAudioForAsr(url, { maxAudioBytes, onProgress, signal })
fetchRemoteAudioForAsr-->>runTranscription: File
runTranscription->>runAsrTranscription: runAsrTranscription(file, normalizeAsrRunSettings(settings), { onUploadProgress, onUploadComplete, onWaitHeartbeat }, { signal })
else input.source == local
runTranscription->>runAsrTranscription: runAsrTranscription(file, normalizeAsrRunSettings(settings), { onUploadProgress, onUploadComplete, onWaitHeartbeat }, { signal })
end
runAsrTranscription-->>runTranscription: AsrTranscriptionResponse
runTranscription-->>useTranscriptionFlow: TranscriptionRunResult
Note over useTranscriptionFlow,runTranscription: shared onState/onLog updates status, progress, result, logs
User-->>useTranscriptionFlow: abort()
useTranscriptionFlow-->>runTranscription: AbortSignal
runTranscription-->>fetchRemoteAudioForAsr: [signal aborts download]
runTranscription-->>runAsrTranscription: [signal aborts ASR request]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the transcription flow by introducing a unified runTranscription runner in lib/transcription-run.ts to handle both local and remote audio sources, streamlining state updates and logging. It also splits settings normalization into independent ASR and LLM slices, and adds abort signal support to remote audio fetching. The review feedback highlights several improvement opportunities: adding defensive checks in settings normalization to handle potential nullish values for backward compatibility, using a more robust check for abort errors to ensure compatibility across different environments, avoiding the Uint8Array<ArrayBuffer> generic type argument to maintain compatibility with older TypeScript versions, and wrapping the runTranscription call in useTranscriptionFlow with a try-catch block to prevent unhandled promise rejections.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
I am having trouble creating individual review comments. Click here to see my feedback.
lib/app-settings.ts (49-60)
为了防止在应用升级时发生运行时崩溃（例如，当从 localStorage 加载旧版本存储的设置时，这些设置可能缺少新引入的 LLM 或 ASR 字段），我们应该在规范化（normalization）过程中防御性地处理可能为 undefined 或 null 的属性。使用可选链（optional chaining）和空值合并运算符（nullish coalescing）可以确保向后兼容性和系统的健壮性。
export const normalizeAsrRunSettings = (input: AsrRunSettings): AsrRunSettings => ({
apiKey: (input?.apiKey ?? '').trim(),
apiUrl: (input?.apiUrl ?? '').trim() || DEFAULT_SETTINGS.apiUrl,
model: (input?.model ?? '').trim() || DEFAULT_SETTINGS.model,
})
export const normalizeLlmRunSettings = (input: LlmRunSettings): LlmRunSettings => ({
llmApiUrl: (input?.llmApiUrl ?? '').trim() || DEFAULT_SETTINGS.llmApiUrl,
llmModel: (input?.llmModel ?? '').trim() || DEFAULT_SETTINGS.llmModel,
llmApiKey: (input?.llmApiKey ?? '').trim(),
customInstructions: (input?.customInstructions ?? '').trim() || DEFAULT_SETTINGS.customInstructions,
})
lib/transcription-run.ts (67-70)
在跨域环境（如 iframe）或某些 Node.js 测试环境中，检查 error instanceof DOMException 可能会失效，因为 DOMException 在这些环境中可能不是全局可用，或者其行为有所不同。此外，一些 fetch polyfill 或第三方库会抛出标准的 Error 对象（其 name 为 'AbortError'）。使用 error instanceof Error && error.name === 'AbortError' 进行检查会更加健壮和通用。
const isAbortError = (error: unknown): boolean =>
error instanceof Error && error.name === 'AbortError'
lib/browser-remote-audio.ts (109)
使用 Uint8Array<ArrayBuffer> 指定了 Uint8Array 的泛型参数，这是 TypeScript 5.0 引入的特性。如果项目或任何开发/CI环境使用的是较旧的 TypeScript 版本，或者配置了较严格的类型检查，这可能会导致编译错误。直接使用标准的 Uint8Array 类型会更安全，且具有更好的兼容性。
const chunks: Array<Uint8Array> = []
lib/browser-remote-audio.ts (119)
同样地，避免在此处使用泛型 Uint8Array<ArrayBuffer> 类型断言，以保持与旧版本 TypeScript 的兼容性。
chunks.push(value as Uint8Array)
hooks/use-transcription-flow.ts (134-149)
虽然 runTranscription 内部捕获了错误并返回了已解析的状态，但在 runTranscription 执行（或其同步初始化）期间，任何意外的同步错误或异常仍可能被抛出。由于 transcribe 是作为一个“触发即忘”（fire-and-forget）的 Promise 被调用的（即 void transcribe(...)），任何未捕获的异常都会导致浏览器中出现未处理的 Promise 拒绝（unhandled promise rejection）。在 runSelectedTranscription 内部将调用包裹在 try-catch 块中，可以恢复健壮的错误处理并防止潜在的页面崩溃。
try {
await runTranscription(input, settings, {
onState: (patch) => {
if (!isCurrentRun()) return
applyTranscriptionRunState(patch)
},
onLog: ({ message, type }) => {
if (!isCurrentRun()) return
addLog(message, type)
},
}, { signal: controller.signal })
} catch (e) {
if (!isCurrentRun()) return
const errorMsg = e instanceof Error ? e.message : String(e)
setFlowError(`请求失败: ${errorMsg}`, '请求失败')
addLog(`请求失败: ${errorMsg}`, 'error')
} finally {
if (!isCurrentRun()) return
transcribeAbortRef.current = null
setLoading(false)
}![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The initialization of
TranscriptionRunState(result/uploadProgress/status/statusMessage) is duplicated betweencreateRunContext, the remote branch inrunTranscription, andrunAsrFileTranscription; consider centralizing the initial state setup in a single helper to avoid subtle divergences in default values over time. - The mutable
requestFailureStatusMessage/requestFailureLogPrefixpair inrunTranscriptioncouples error handling flow in a way that’s a bit hard to follow; you could make the remote and local paths each return a more explicit error-context object (or wrap the ASR call in a higher-order helper) to keep the error mapping logic more declarative.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The initialization of `TranscriptionRunState` (result/uploadProgress/status/statusMessage) is duplicated between `createRunContext`, the remote branch in `runTranscription`, and `runAsrFileTranscription`; consider centralizing the initial state setup in a single helper to avoid subtle divergences in default values over time.
- The mutable `requestFailureStatusMessage` / `requestFailureLogPrefix` pair in `runTranscription` couples error handling flow in a way that’s a bit hard to follow; you could make the remote and local paths each return a more explicit error-context object (or wrap the ASR call in a higher-order helper) to keep the error mapping logic more declarative.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Bot review 处理进度：
本地验证：npm test 通过；npm run build 通过。 |
|
Bot review 处理完毕：
本地验证：npm test 通过；npm run build 通过。 |
8c68010 to
1a9f961
Compare
3255cd0 to
a951161
Compare
1 participant
对应 Issue
Closes #26
变更
runTranscription，本地文件和远程链接都产出同一套 status/progress/result/log/abort 语义。useTranscriptionFlow改为只把 local/remote input 交给 run module，不再维护两套转录运行逻辑。验证
npm test -- --run tests/unit/transcription-run.test.ts tests/unit/browser-remote-audio.test.ts tests/unit/asr-transcription.test.tsnpm testnpm run build依赖
Summary by Sourcery
Unify local file and remote URL transcription flows into a single run pipeline and decouple ASR and LLM runtime settings normalization.
New Features:
Enhancements:
Tests: