feat(dub): wire second-pass timing QC into the dub editor UI - #458
Conversation
The Wave 3.3 QC backend was complete but unreachable from the UI: the
`POST /dub/qc/{job_id}` route (re-recognizes the dubbed audio, scores per-line
drift vs the target text, annotates segments with qc_drift/qc_flagged/
qc_recognized/qc_measured_start-end), the `dubQc()` API client, and the
DubSegmentRow "Verify" badge all existed — but nothing ever called the route,
so the badge never lit and the measured timings were never surfaced.
Add a "Verify dub timing" action to the dub editor header (shown once
dubStep === 'done'):
- Calls `dubQc(jobId, lang)` for the currently-previewed language.
- Merges the returned per-segment scores back onto dubSegments by id, so
flagged lines light their re-listen badge and carry the measured onsets.
- Toast summary: "{flagged} of {total} lines may need a re-listen", or a
clean-pass success when nothing drifted. Loading + error states handled;
non-destructive (generated text untouched).
i18n: dub.qc_btn / qc_running / qc_result / qc_clean / qc_failed in en.json
(fallbackLng=en covers other locales). Frontend suite green (401).
📝 WalkthroughWalkthroughAdds a second-pass dub timing QC feature to ChangesDub Timing Second-Pass QC
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Panel notes (no filler):
🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
| Filename | Overview |
|---|---|
| frontend/src/pages/DubTab.jsx | Adds handleDubQc callback and 'Verify dub timing' button; stale-closure on dubSegments can silently overwrite concurrent user edits when the async ASR pass returns. |
| frontend/src/i18n/locales/en.json | Adds five new i18n keys under the dub namespace for the QC button and result toasts; keys and interpolation variables are consistent with usage in DubTab.jsx. |
Sequence Diagram
sequenceDiagram
participant U as User
participant DT as DubTab (React)
participant BE as Backend sidecar
U->>DT: "Click Verify dub timing (dubStep=done)"
DT->>DT: setQcRunning(true) toast.loading
DT->>BE: "POST /dub/qc/{jobId}?lang={previewMode or undefined}"
Note over BE: ASR re-recognition pass on dubbed audio (seconds)
BE-->>DT: DubQCResponse segments[] flagged_count total
DT->>DT: build byId map from seg_id
Note over DT: merges into closure-snapshot of dubSegments
DT->>DT: setDubSegments(stale.map merge QC fields)
alt "flagged_count > 0"
DT-->>U: Toast N of M lines may need a re-listen
else
DT-->>U: Toast All N lines match the script
end
DT->>DT: setQcRunning(false)
Reviews (1): Last reviewed commit: "feat(dub): wire second-pass timing QC in..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const byId = new Map((res.segments || []).map(q => [String(q.seg_id), q])); | ||
| setDubSegments(dubSegments.map((s, i) => { | ||
| const q = byId.get(String(s.id ?? i)); | ||
| if (!q) return s; | ||
| return { | ||
| ...s, | ||
| qc_drift: q.drift, | ||
| qc_flagged: q.flagged, | ||
| qc_recognized: q.recognized_text, | ||
| ...(q.measured_start != null ? { qc_measured_start: q.measured_start, qc_measured_end: q.measured_end } : {}), | ||
| }; | ||
| })); |
There was a problem hiding this comment.
dubSegments is read from the callback's closure, which was snapshotted at memoization time. The ASR pass on the dubbed audio takes several seconds; during that window the user can still edit individual segment text or timings (the segment table is not locked). When setDubSegments(dubSegments.map(...)) runs on return, it applies QC deltas onto the stale snapshot, silently discarding any edits made while QC was in flight. Using the functional-updater form (prev => prev.map(...)) always operates on the latest committed state and also lets dubSegments be removed from the useCallback dep array, preventing unnecessary re-memoizations on every keystroke.
| const byId = new Map((res.segments || []).map(q => [String(q.seg_id), q])); | |
| setDubSegments(dubSegments.map((s, i) => { | |
| const q = byId.get(String(s.id ?? i)); | |
| if (!q) return s; | |
| return { | |
| ...s, | |
| qc_drift: q.drift, | |
| qc_flagged: q.flagged, | |
| qc_recognized: q.recognized_text, | |
| ...(q.measured_start != null ? { qc_measured_start: q.measured_start, qc_measured_end: q.measured_end } : {}), | |
| }; | |
| })); | |
| const byId = new Map((res.segments || []).map(q => [String(q.seg_id), q])); | |
| setDubSegments(prev => prev.map((s, i) => { | |
| const q = byId.get(String(s.id ?? i)); | |
| if (!q) return s; | |
| return { | |
| ...s, | |
| qc_drift: q.drift, | |
| qc_flagged: q.flagged, | |
| qc_recognized: q.recognized_text, | |
| ...(q.measured_start != null ? { qc_measured_start: q.measured_start, qc_measured_end: q.measured_end } : {}), | |
| }; | |
| })); |
| } finally { | ||
| setQcRunning(false); | ||
| } | ||
| }, [dubJobId, qcRunning, previewMode, dubSegments, setDubSegments, t]); |
There was a problem hiding this comment.
dubSegments no longer needs to be in the dependency array — the updater receives the current state directly from React rather than from the closure. Keeping it in deps means handleDubQc is re-memoized on every keystroke a user makes to a segment. qcRunning can also be removed because the early-return guard at the top of the callback is read synchronously at call time — including it in deps just creates a second memoization when QC toggles on/off without changing behaviour.
| }, [dubJobId, qcRunning, previewMode, dubSegments, setDubSegments, t]); | |
| }, [dubJobId, previewMode, setDubSegments, t]); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| setQcRunning(true); | ||
| const loadingId = toast.loading(t('dub.qc_running', { defaultValue: 'Checking dub timing…' })); | ||
| try { | ||
| const lang = previewMode !== 'original' ? previewMode : undefined; |
There was a problem hiding this comment.
lang is set to undefined whenever the user is previewing the original audio, so POST /dub/qc/{job_id} is sent without a lang parameter. In a multi-language dub job the backend has no signal about which track to recognize, and the QC scores it returns will be merged onto dubSegments with segment IDs that could belong to any track. The resulting qc_flagged annotations would be misleading or simply wrong for the language the user actually intends to verify. Consider either (a) requiring a specific dubbed track to be selected before QC is enabled, or (b) deriving lang from dubLangCode (the active job language) rather than previewMode.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/i18n/locales/en.json`:
- Around line 653-657: The five new i18n keys (qc_btn, qc_running, qc_result,
qc_clean, qc_failed) added to en.json are missing from the other 20 locale
files. Add these same five keys with appropriate translations to all 20 locale
files: ar.json, bn.json, de.json, es.json, fr.json, hi.json, id.json, it.json,
ja.json, ko.json, nl.json, pl.json, pt.json, ru.json, sv.json, th.json, tr.json,
uk.json, vi.json, zh-CN.json, and zh-TW.json. Ensure each key is added under the
same nested structure (dub object) with translations matching the English values
or appropriate language-specific translations. This ensures all locales have the
required keys and prevents untranslated English strings from appearing to
non-English users.
In `@frontend/src/pages/DubTab.jsx`:
- Around line 339-380: The handleDubQc callback has a stale closure issue with
dubSegments. When setDubSegments is called at line 350, it reads the dubSegments
value from the closure which may be outdated if segments changed between
callback definition and execution. This overwrites user edits with stale data.
Fix this by changing the setDubSegments call to use the functional update form:
pass a function that receives the current state as a parameter instead of
reading dubSegments from the closure. Then remove dubSegments from the
useCallback dependency array on line 380 since it is no longer directly
referenced in the closure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f3eaa9d5-9ce7-4da1-a730-f61150c8f356
📒 Files selected for processing (2)
frontend/src/i18n/locales/en.jsonfrontend/src/pages/DubTab.jsx
| "qc_btn": "Verify dub timing (second-pass check)", | ||
| "qc_running": "Checking dub timing…", | ||
| "qc_result": "{{flagged}} of {{total}} lines may need a re-listen", | ||
| "qc_clean": "All {{total}} lines match the script", | ||
| "qc_failed": "Timing check failed: {{message}}", |
There was a problem hiding this comment.
Five new i18n keys added, but only to en.json — missing from the other 20 locale files.
The coding guideline requires: "For every new or changed t('...') key in frontend code, verify the key exists in all 21 files under frontend/src/i18n/locales/." These five keys (dub.qc_btn, qc_running, qc_result, qc_clean, qc_failed) must be added to all 21 locale files:
ar.json, bn.json, de.json, es.json, fr.json, hi.json, id.json, it.json, ja.json, ko.json, nl.json, pl.json, pt.json, ru.json, sv.json, th.json, tr.json, uk.json, vi.json, zh-CN.json, zh-TW.json.
With fallbackLng=en the app won't crash, but users in non-English locales will see untranslated English strings for QC features.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/i18n/locales/en.json` around lines 653 - 657, The five new i18n
keys (qc_btn, qc_running, qc_result, qc_clean, qc_failed) added to en.json are
missing from the other 20 locale files. Add these same five keys with
appropriate translations to all 20 locale files: ar.json, bn.json, de.json,
es.json, fr.json, hi.json, id.json, it.json, ja.json, ko.json, nl.json, pl.json,
pt.json, ru.json, sv.json, th.json, tr.json, uk.json, vi.json, zh-CN.json, and
zh-TW.json. Ensure each key is added under the same nested structure (dub
object) with translations matching the English values or appropriate
language-specific translations. This ensures all locales have the required keys
and prevents untranslated English strings from appearing to non-English users.
Source: Coding guidelines
| // Second-pass timing QC (Wave 3.3): re-recognize the dubbed audio and merge | ||
| // the per-line drift scores back onto the segments so DubSegmentRow can flag | ||
| // lines worth a re-listen. Non-destructive — generated text is untouched. | ||
| const handleDubQc = useCallback(async () => { | ||
| if (!dubJobId || qcRunning) return; | ||
| setQcRunning(true); | ||
| const loadingId = toast.loading(t('dub.qc_running', { defaultValue: 'Checking dub timing…' })); | ||
| try { | ||
| const lang = previewMode !== 'original' ? previewMode : undefined; | ||
| const res = await dubQc(dubJobId, lang); | ||
| const byId = new Map((res.segments || []).map(q => [String(q.seg_id), q])); | ||
| setDubSegments(dubSegments.map((s, i) => { | ||
| const q = byId.get(String(s.id ?? i)); | ||
| if (!q) return s; | ||
| return { | ||
| ...s, | ||
| qc_drift: q.drift, | ||
| qc_flagged: q.flagged, | ||
| qc_recognized: q.recognized_text, | ||
| ...(q.measured_start != null ? { qc_measured_start: q.measured_start, qc_measured_end: q.measured_end } : {}), | ||
| }; | ||
| })); | ||
| if (res.flagged_count > 0) { | ||
| toast(t('dub.qc_result', { | ||
| flagged: res.flagged_count, total: res.total, | ||
| defaultValue: '{{flagged}} of {{total}} lines may need a re-listen', | ||
| }), { icon: '⚠️', id: loadingId, duration: 6000 }); | ||
| } else { | ||
| toast.success(t('dub.qc_clean', { | ||
| total: res.total, | ||
| defaultValue: 'All {{total}} lines match the script', | ||
| }), { id: loadingId }); | ||
| } | ||
| } catch (err) { | ||
| toast.dismiss(loadingId); | ||
| toastErrorWithReport( | ||
| t('dub.qc_failed', { message: String(err?.message || err).slice(0, 200), | ||
| defaultValue: 'Timing check failed: {{message}}' }), err); | ||
| } finally { | ||
| setQcRunning(false); | ||
| } | ||
| }, [dubJobId, qcRunning, previewMode, dubSegments, setDubSegments, t]); |
There was a problem hiding this comment.
Stale closure on dubSegments — can overwrite user edits with old segment state.
Line 350 reads dubSegments from the callback's closure, then line 350 calls setDubSegments(dubSegments.map(...)). If segments change between callback definition and execution (user edits, adds, or removes segments while QC is running), the merge overwrites the current state with a modified stale array, losing the user's edits.
Use the functional update form so the merge always operates on the latest state:
🔒 Proposed fix
- setDubSegments(dubSegments.map((s, i) => {
- const q = byId.get(String(s.id ?? i));
+ setDubSegments(prev => prev.map((s, i) => {
+ const q = byId.get(String(s.id ?? i));
if (!q) return s;
return {
...s,
qc_drift: q.drift,
qc_flagged: q.flagged,
qc_recognized: q.recognized_text,
...(q.measured_start != null ? { qc_measured_start: q.measured_start, qc_measured_end: q.measured_end } : {}),
};
}));Then remove dubSegments from the useCallback dependency array (line 380) — it's no longer closed over.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/DubTab.jsx` around lines 339 - 380, The handleDubQc
callback has a stale closure issue with dubSegments. When setDubSegments is
called at line 350, it reads the dubSegments value from the closure which may be
outdated if segments changed between callback definition and execution. This
overwrites user edits with stale data. Fix this by changing the setDubSegments
call to use the functional update form: pass a function that receives the
current state as a parameter instead of reading dubSegments from the closure.
Then remove dubSegments from the useCallback dependency array on line 380 since
it is no longer directly referenced in the closure.
1 participant
Closes the polished dubbing — second-pass timing QC gap from discussion #346. The Wave 3.3 QC backend shipped complete but was unreachable from the UI: the
POST /dub/qc/{job_id}route (re-recognizes the dubbed audio, scores per-line drift vs target viaservices.dub_qc, annotates segments withqc_drift/qc_flagged/qc_recognized/qc_measured_start-end), thedubQc()client, and theDubSegmentRow"Verify" badge all existed — but nothing called the route, so the badge never lit.Change
A "Verify dub timing" action in the dub editor header (shown once
dubStep === 'done'):dubQc(jobId, lang)for the currently-previewed language.dubSegmentsby id → flagged lines light their re-listen badge and carry the measured onsets.Tests / checks
Backend QC scoring already covered by
tests/test_dub_qc.py. Frontend suite green (401 passed);en.jsonvalid. i18n keys added (dub.qc_btn/qc_running/qc_result/qc_clean/qc_failed);fallbackLng=encovers other locales.🤖 Generated with Claude Code
Internationalization (i18n)
Added five new localization strings to
frontend/src/i18n/locales/en.jsonunder thedubgroup to support the QC verification feature:qc_btn: "Verify dub timing (second-pass check)"qc_running: "Checking dub timing…"qc_result: "{{flagged}} of {{total}} lines may need a re-listen"qc_clean: "All {{total}} lines match the script"qc_failed: "Timing check failed: {{message}}"These keys provide user-facing text for QC triggering, progress indication, and result reporting (both flagged and success cases).
UI Changes
The dub editor header bar gains a new "Verify dub timing" button that integrates the Wave 3.3 QC backend:
The button displays a
ShieldCheckicon, swaps to a loading spinner when QC is running, and is disabled until dubbing completes and segments exist. It only appears whendubStep === 'done'.QC Verification Flow
graph TD A["User clicks 'Verify dub timing'"] --> B["handleDubQc() triggered"] B --> C["Determine language:<br/>(previewMode or undefined)"] C --> D["dubQc(jobId, lang)<br/>(API call)"] D --> E["Show toast:<br/>'Checking dub timing…'"] F["Backend returns:<br/>segments with QC scores"] --> G["Build QC map<br/>by segment ID"] G --> H["Merge results:<br/>qc_drift, qc_flagged,<br/>qc_recognized,<br/>qc_measured_start/end"] H --> I{Flagged lines?} I -->|Yes| J["Toast warning:<br/>'N of M lines<br/>may need re-listen'"] I -->|No| K["Toast success:<br/>'All M lines<br/>match script'"] L["Error caught"] --> M["Dismiss loading toast"] M --> N["toastErrorWithReport"] O["Finally: setQcRunning false"] D --> F D --> L J --> O K --> O N --> OImplementation Details
In
DubTab.jsx:Imported
dubQcfrom the dub API clientAdded
ShieldCheckicon importAdded
qcRunningstate to track QC executionImplemented
handleDubQccallback that:dubQc(dubJobId, lang)for the current preview language (or undefined for original/all tracks)qc_drift,qc_flagged,qc_recognized, measured timings) back intodubSegmentsAdded footer button visible only when
dubStep === 'done', disabled whenqcRunningor no segments existButton displays spinner icon during QC execution, reverts to
ShieldCheckwhen idleThe implementation is non-destructive: QC results enrich segment metadata without modifying the actual dubbed text or translations, maintaining consistency with the design approach.