Skip to content

fix(dubbing): score canonically equivalent Unicode text consistently - #2694

Open
rudycelekli wants to merge 3 commits into
debpalash:mainfrom
rudycelekli:fix/dub-qc-canonical-unicode-20261010
Open

rudycelekli wants to merge 3 commits into
debpalash:mainfrom
rudycelekli:fix/dub-qc-canonical-unicode-20261010

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #2693.

Dub quality scoring now treats canonically equivalent Unicode spellings as the same text. A composed ASR spelling no longer causes a verification flag when a subtitle uses the equivalent combining characters.

Changes

  • Normalize scoring copies to NFC before tokenization; preserve stored target and recognized text.
  • Cover both spelling directions through the actual UTF-8 upload decoder, subtitle parser and QC scorer for Latin, Japanese and Korean text.
  • Preserve genuine accent/word differences, compatibility ligatures and measured timing.
  • Document the scoring contract and add the referenced changelog entry.
  • Describe the two new regression test contracts in docstrings, addressing the review warning.

Type

  • 🐛 Bug fix
  • ✨ New feature
  • ♻️ Refactor
  • 📝 Documentation
  • 🧪 Tests
  • 🔧 CI / Build
  • 🚀 Release prep

Testing

  • Python 3.11 native upload/parser/QC consumer: six false-drift cases before the fix, zero after.
  • Frozen QC regressions: seven failures and seventeen passes before; all twenty-four pass after.
  • Forty-four focused QC, changelog and CJK policy checks pass.
  • All unchanged fork CI jobs passed on the implementation commit d54ae7f2a478bab4d103374806a7b32542b86047: full tests/ (10,439 passed; 31 skipped; 8 xfailed; 1 xpassed), isolated backend/tests/ (512 passed), frontend units, Electron typecheck/tests/production build, renderer checks and platform smokes. The supported macOS/Windows/Linux jobs load the frozen fixture; the Intel Mac job checks the documented unsupported local-backend contract.
  • Security, install smoke, CosyVoice dependencies and native builds pass on the exact candidate tree. Report-only security scans retain findings outside the changed scoring files; workflow success does not mean the repository is free of findings.
  • Local full-suite/Electron attempts failed due missing dependencies; the local passes above cover the focused scorer and policy checks.
  • Supplied recognition segments exercise deterministic scoring. No model-backed ASR, dubbing or audio-quality result is claimed.

Hosted validation

CI, Security, install smoke, CosyVoice dependencies, native builds. The fork validation merge has exactly the implementation tree at d54ae7f2a478bab4d103374806a7b32542b86047. Current head 8b0762404d6158e7d80925f447e5218f85a39a2e adds only two test docstrings; its executable test AST and runtime source are unchanged. All twenty-four QC tests pass on Python 3.13.5 after that documentation change. All upstream checks on this latest signed head have completed successfully, including the full test gate and platform smoke matrix: current upstream CI. Report-only security findings and model/inference limits remain as described above.

Docker qualification

The unchanged default deploy/Dockerfile built successfully for CUDA linux/amd64 in an isolated build-only fork run, using implementation commit d54ae7f2a478bab4d103374806a7b32542b86047. The latest commit changes only test docstrings; no Dockerfile, dependency, copied application or frontend input changes. An offline check inside the built image verified the scoring source hash, six canonical-equivalence directions and three genuine-difference controls. The image was not published. ROCm was not built; no GPU inference, ASR/TTS or audio-quality result is claimed.

Checklist

  • I've tested this locally (focused scope above)
  • Every commit author has an existing CLA registration
  • I've updated relevant documentation
  • No local machine paths, logs, or personal env details in this PR
  • Maintained version files are unchanged (no owner-requested version bump)
  • The frozen data fixture loads green on the supported macOS, Windows and Linux smoke matrix

The dub QC scorer now NFC-normalizes text before tokenization, without changing stored text or measured timing. This prevents false drift for canonically equivalent Unicode spellings while preserving accent, word, and compatibility-character differences. No material merge risk is evident from the supplied changes.

Release cadence

This is a bug-fix contribution to continuous main delivery. No version bump or release publication is requested. Merging, packaging and publishing remain subject to maintainer review and the repository release checklist.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium impact] The reviewed changes appear safe to merge; no blocking issue was found.

Summary

The PR normalizes text to NFC before scoring and adds upload-to-QC regression coverage.

  • Dub quality checks give equivalent Unicode spellings the same score.

Reviews (3) · Last reviewed commit: "Merge current main and preserve Unicode ..." · Reviewed by Greptile

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: debpalash/VoiceStudio/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c2eb8341-36ec-4cdf-8c7b-f8af99860b7d


📥 Commits

Reviewing files that changed from the base of the PR and between 8b07624 and 4815ff4.



📒 Files selected for processing (2)
  • CHANGELOG.md
  • backend/services/dub_qc.py


🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md


Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.




📝 Walkthrough
📝 Walkthrough

Walkthrough

Dub QC applies NFC normalization before token comparison. Canonically equivalent spellings score as equivalent without changing stored text or recognition timings. Tests cover canonical equivalents and differences in accents, kana, words, and compatibility characters.

Changes

Unicode-equivalent dub QC scoring

Layer / File(s) Summary
NFC token normalization and scoring coverage
backend/services/dub_qc.py, tests/test_dub_qc.py, docs/dubbing/quality-control.md, CHANGELOG.md
Dub QC normalizes tokens to NFC before comparison. Tests check equivalent spellings and verify that differences in accents, kana, words, and compatibility characters remain distinct. Documentation and the changelog describe the behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: debpalash



Merge Risk: ⚪ Minimal · up to 4815f

The change addresses false drift for canonically equivalent text, and the added tests cover the upload-to-scoring path while checking that source text and timing remain intact. No unresolved merge risk is established.

Pre-merge checks | Passed 7 | Failed 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check Warning The current head adds model-licence acceptance and routing changes across audiobook, batch, capture, WebSocket transcription, and dubbing endpoints, plus #2689 licensing changes in README.md, LICENSE-… Remove the unrelated model-licence and #2689 changes from this pull request, or move them to a separate pull request.
Docstring Coverage Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (7 passed)
Check name Status Explanation
Linked Issues check Passed Issue #2693 requires zero drift for canonically equivalent spellings while preserving stored text, timing, genuine differences, and compatibility-character behavior. The reviewed scoring change normal…
Cross-Platform Default Parity Passed The PR changes the default QC scoring path, but the changed operation is platform-independent. backend/services/dub_qc.py:57 uses Python unicodedata.normalize("NFC", ...), lowercasing, and the exi…
I18n Completeness (21 Locales) Passed PASS — The pull request changes only CHANGELOG.md, backend/services/dub_qc.py, docs/dubbing/quality-control.md, and tests/test_dub_qc.py. It changes no Electron UI code, adds no t('...') keys, and int…
Local-First Guarantee Passed The PR changes only local Unicode scoring, tests, and documentation. backend/services/dub_qc.py adds unicodedata.normalize inside a documented pure, no-I/O scorer; it adds no network, account, API…
Backward Compatibility Passed The pull request changes only Unicode normalization in the pure backend/services/dub_qc.py scoring path, plus tests and documentation. The diff contains no database schema, Alembic migration, depend…
Title check Passed The title uses Conventional Commit format with the required scope and accurately describes the Unicode scoring fix. The issue reference appears in the pull request body as #2693.
Description check Passed The description includes the required summary, changes, type, testing, checklist, and release cadence sections. It documents the fix, validation results, known test limitations, and release impact.

Full details: Out of Scope Changes check

Explanation

The current head adds model-licence acceptance and routing changes across audiobook, batch, capture, WebSocket transcription, and dubbing endpoints, plus #2689 licensing changes in README.md, LICENSE-NOTICE.md, and CHANGELOG.md. These changes do not implement Unicode normalization or dub QC behavior for issue #2693.


Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.)


  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@rudycelekli

Copy link
Copy Markdown
Contributor Author

Merged current main into the PR branch and resolved the changelog conflict by preserving both the Unicode QC entry and the new model-licence notices. The scoring code and regression tests are unchanged. The focused QC and changelog suite passes (37 tests); full CI for the updated revision 4815ff47 is running. The previous completed runs covered the earlier revision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dub QC flags canonically equivalent Unicode text as different speech

1 participant