Repository navigation
Deepen local transcription run module - #33
Merged
Merged
Conversation
Reviewer's GuideExtracts local ASR run orchestration into a reusable run module and simplifies the React transcription flow hook to just wiring and state updates, with comprehensive unit tests for run-level behavior. Sequence diagram for the new local transcription run orchestrationsequenceDiagram
actor User
participant ReactComponent
participant useTranscriptionFlow
participant runLocalTranscription
participant runAsrTranscription
participant AsrApi
User ->> ReactComponent: clickTranscribe(file)
ReactComponent ->> useTranscriptionFlow: transcribe(file)
useTranscriptionFlow ->> useTranscriptionFlow: prepareRun()
useTranscriptionFlow ->> runLocalTranscription: runLocalTranscription(file, settings, callbacks, options)
runLocalTranscription ->> runLocalTranscription: normalizeAsrRunSettings(settings)
runLocalTranscription ->> useTranscriptionFlow: callbacks.onState(initialState)
runLocalTranscription ->> useTranscriptionFlow: callbacks.onLog(info logs)
runLocalTranscription ->> runAsrTranscription: runAsrTranscription(file, effectiveSettings, handlers, options)
runAsrTranscription ->> AsrApi: HTTP upload + transcribe
loop upload
AsrApi -->> runAsrTranscription: progress events
runAsrTranscription ->> runLocalTranscription: onUploadProgress(progress)
runLocalTranscription ->> useTranscriptionFlow: callbacks.onState({ uploadProgress, statusMessage })
runLocalTranscription ->> useTranscriptionFlow: callbacks.onLog(upload progress)
end
runAsrTranscription ->> runLocalTranscription: onUploadComplete()
runLocalTranscription ->> useTranscriptionFlow: callbacks.onState({ status: transcribing, statusMessage })
runLocalTranscription ->> useTranscriptionFlow: callbacks.onLog(upload complete)
loop heartbeat
AsrApi -->> runAsrTranscription: wait heartbeat
runAsrTranscription ->> runLocalTranscription: onWaitHeartbeat(heartbeat)
runLocalTranscription ->> useTranscriptionFlow: callbacks.onState({ statusMessage })
runLocalTranscription ->> useTranscriptionFlow: callbacks.onLog(wait log)
end
runAsrTranscription -->> runLocalTranscription: response
alt response.ok && text
runLocalTranscription ->> useTranscriptionFlow: callbacks.onState({ result: success, status: done })
runLocalTranscription ->> useTranscriptionFlow: callbacks.onLog(success log)
else response.ok && empty text
runLocalTranscription ->> useTranscriptionFlow: callbacks.onState({ result: empty, status: done })
runLocalTranscription ->> useTranscriptionFlow: callbacks.onLog(empty log)
else !response.ok
runLocalTranscription ->> useTranscriptionFlow: callbacks.onState({ result: error, status: error })
runLocalTranscription ->> useTranscriptionFlow: callbacks.onLog(API error log)
end
runLocalTranscription -->> useTranscriptionFlow: LocalTranscriptionRunResult
useTranscriptionFlow ->> ReactComponent: update React state
ReactComponent -->> User: updated UI
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
More reviews will be available in 45 minutes and 1 second. 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 (3)
✨ 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 |
![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 extracting the core ASR transcription logic from the useTranscriptionFlow hook into a dedicated helper function runLocalTranscription in lib/local-transcription-run.ts, accompanied by unit tests. The reviewer feedback highlights two key areas for improvement: first, adding a catch block back to the transcribe function in the hook to prevent unhandled promise rejections from unexpected runtime or callback errors; second, enhancing the robustness of the isAbortError helper to support environments where abort errors are standard Error objects rather than DOMException instances.
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.
![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 found 3 issues, and left some high level feedback:
- In
useTranscriptionFlow.transcribe, the result ofrunLocalTranscriptionis ignored; if the UI or error banners rely onsetFlowError, consider mappingrunLocalTranscriptionerror states back intoflowErrorto preserve the previous error‑handling behavior. beginRuninuseTranscriptionFlowis now unused after introducingprepareRunandrunLocalTranscription; consider removing it (or inlining its remaining behavior) to avoid dead code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `useTranscriptionFlow.transcribe`, the result of `runLocalTranscription` is ignored; if the UI or error banners rely on `setFlowError`, consider mapping `runLocalTranscription` error states back into `flowError` to preserve the previous error‑handling behavior.
- `beginRun` in `useTranscriptionFlow` is now unused after introducing `prepareRun` and `runLocalTranscription`; consider removing it (or inlining its remaining behavior) to avoid dead code.
## Individual Comments
### Comment 1
<location path="lib/local-transcription-run.ts" line_range="43-52" />
<code_context>
+ let state: LocalTranscriptionRunState = {
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid emitting multiple synchronous state updates before the first await to reduce unnecessary re-renders.
`runLocalTranscription` currently calls `emitState(state)` and then `emitState({ statusMessage: '正在上传文件...' })` before any `await`, causing two near-identical `onState` callbacks and re-renders for React consumers. Instead, set `statusMessage: '正在上传文件...'` in the initial `state` or include it in the first `emitState` call so the initial render happens only once.
Suggested implementation:
```typescript
let state: LocalTranscriptionRunState = {
result: IDLE_TRANSCRIPTION_RESULT,
uploadProgress: 0,
status: 'processing',
statusMessage: '正在上传文件...',
}
```
```typescript
emitState(state)
```
I assumed there is an initial `emitState(state)` followed by `emitState({ statusMessage: '正在上传文件...' })` as described in your comment. If the second call has slightly different text or is wrapped in additional logic, adjust the second SEARCH block to match the actual code and remove only that extra `emitState` invocation while keeping the first one.
</issue_to_address>
### Comment 2
<location path="lib/local-transcription-run.ts" line_range="33-34" />
<code_context>
+ kind: 'success' | 'empty' | 'error' | 'aborted'
+}
+
+const isAbortError = (error: unknown): boolean =>
+ error instanceof DOMException && error.name === 'AbortError'
+
+export const runLocalTranscription = async (
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against environments where DOMException is not defined when detecting abort errors.
`isAbortError` assumes `DOMException` is always defined, which can cause a `ReferenceError` in non-browser or older environments. Guard the check to avoid that runtime dependency, e.g.:
```ts
const isAbortError = (error: unknown): boolean =>
typeof DOMException !== 'undefined' &&
error instanceof DOMException &&
(error as DOMException).name === 'AbortError'
```
</issue_to_address>
### Comment 3
<location path="tests/unit/local-transcription-run.test.ts" line_range="68" />
<code_context>
+ }
+}
+
+describe('runLocalTranscription', () => {
+ it('emits start, upload, heartbeat, and success state for a local ASR run', async () => {
+ const xhr = new FakeXMLHttpRequest()
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a smoke test where `runLocalTranscription` is called without any callbacks to ensure optional handlers don’t cause runtime errors.
All current tests use `collectRunEvents`, so callbacks are always present. Since `LocalTranscriptionRunCallbacks` is optional, add a small case like `it('handles missing callbacks without throwing', ...)` that calls `runLocalTranscription(file, settings)` without the third argument and asserts the promise resolves to the expected success shape. This will catch regressions where `callbacks.onState`/`callbacks.onLog` are used without null-checks.
Suggested implementation:
```typescript
describe('runLocalTranscription', () => {
it('handles missing callbacks without throwing', async () => {
const resultPromise = runLocalTranscription(buildFile(), buildSettings())
await expect(resultPromise).resolves.toEqual(expect.anything())
})
```
If `runLocalTranscription` requires injected dependencies (e.g., a `FakeXMLHttpRequest` via an options argument) to run in tests, adapt this smoke test to match the pattern of the existing tests, for example:
1. Construct a `FakeXMLHttpRequest` and any required timing utilities (`now`, `setIntervalFn`, etc.).
2. Call `runLocalTranscription(buildFile(), buildSettings(), undefined, { /* test options mirroring other tests */ })` so that the callbacks parameter is explicitly `undefined` while still providing necessary options.
3. Drive the fake XHR to completion in the same way the success test does, if the promise does not resolve on its own.
Adjust the `expect` assertion to whatever “success” value the other tests expect (e.g., `expect.objectContaining({ status: 'success' })`) to keep the contract consistent across tests.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
Bot review 处理记录：
验证： |
makoMakoGo
force-pushed
the
codex/local-transcription-run-module
branch
from
June 10, 2026 09:38
b0e99f9 to
320e148
Compare
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
1 participant
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.
关联 issue
Refs #20
依赖关系
这个 PR 叠在 #30 (
codex/runtime-settings-slices) 之上，因为本地转录运行模块直接使用 #30 拆出的AsrRunSettings运行切片。它和 #32（润色运行模块）互不依赖，都是 #30 之后可以独立 review 的兄弟 PR。#30 合并后，本 PR 再 rebase/retarget 到
main。改动内容
lib/local-transcription-run.ts，集中一次本地音频 ASR 转录运行的状态、上传进度、等待识别心跳、成功、空结果、API 错误、请求失败和取消语义。hooks/use-transcription-flow.ts的本地转录路径，让 hook 只负责清日志、重置 UI 外壳、AbortController 当前运行隔离，以及把 run module 的 state/log patch 落到 React state。tests/unit/local-transcription-run.test.ts，覆盖 run-level 的 status、progress、heartbeat、abort、result 和错误语义。tests/unit/asr-transcription.test.ts对底层 XHR 请求构造和上传细节的覆盖。验证
npm test -- --run tests/unit/local-transcription-run.test.ts tests/unit/asr-transcription.test.ts tests/unit/utils.test.tsnpm testnpm run buildSummary by Sourcery
Extract local ASR transcription run logic into a reusable module and wire it into the transcription flow hook while adding focused unit coverage.
Enhancements:
Tests: