Skip to content

fix(cli): keep phrase-level CJK and Thai transcripts as separate cues - #3436

Merged
miga-heygen merged 2 commits into
heygen-com:mainfrom
rajanpanth:fix/cjk-phrase-level-cues
Aug 26, 2026
Merged

fix(cli): keep phrase-level CJK and Thai transcripts as separate cues#3436
miga-heygen merged 2 commits into
heygen-com:mainfrom
rajanpanth:fix/cjk-phrase-level-cues

Conversation

@rajanpanth

Copy link
Copy Markdown
Contributor

Fixes #3353

Problem

wordsToCues decided whether its input was already grouped into phrases by testing for internal whitespace:

const preGrouped = opts.preGrouped ?? words.some((w) => /\s/.test(w.text.trim()));

Chinese, Japanese and Thai do not put spaces between words, so for those scripts the test is always false. Phrase-level entries were treated as individual words and re-grouped into one cue covering the whole transcript. Three Chinese phrases produced one cue; the same three phrases in English produced three.

The failure was silent, which is what made it expensive: as the issue records, two projects hit it and each built their own pipeline rather than finding --preserve-cues.

Approach

The issue lists three options. I took the second, keeping the whitespace test and adding a codepoint check alongside it, but with the length signal from the first, because a codepoint check on its own is not enough to decide the question.

Whether entries are pre-grouped is really "does an entry hold more than one token". Whitespace answers that for space-separated scripts. For spaceless scripts nothing in the codepoints answers it: word-level whisper output and phrase-level cues are both unbroken runs of Han characters. Treating every CJK transcript as pre-grouped would emit one cue per token and break the normal transcribe path, which the existing "joins CJK word-level tokens without inserting spaces" test covers.

So for spaceless scripts the fallback is entry length. Whisper emits word-level tokens for those scripts one or two characters at a time, while a phrase-level cue runs to several times that, and four sits comfortably between.

The median is used rather than some or every: some would let one long token declare word-level input pre-grouped, and every would let a couple of short cues (a bare yes or no) declare a real transcript word-level.

I did not take option 1 wholesale. Replacing the whitespace test with a duration heuristic for all scripts would put every existing English transcript through a new classifier, and this change deliberately leaves space-separated input on exactly the path it is on today.

Detection also covers Thai, Lao, Myanmar and Khmer, which have the same problem. I left joinTokens and its CJK_CHAR alone: widening the separator rule is a real change to output for those scripts and belongs in its own PR.

Testing

Added to normalize.test.ts:

  • three phrase-level Chinese entries produce three cues with their original timings, the issue's repro
  • two phrase-level Thai entries produce two cues
  • four short word-level CJK tokens still group into one cue, the regression guard for the case above

The existing "joins CJK word-level tokens without inserting spaces" test passes unchanged: its tokens have a median length of 1, well under the threshold.

Reverting the inferPreGrouped call while keeping the tests fails the CJK and Thai cases, so they cover the change rather than restating current behaviour.

bunx vitest run packages/cli/src/whisper/ passes at 106 tests with no pre-existing failures, and bunx oxlint / bunx oxfmt --check are clean on both files.

wordsToCues inferred whether entries were already grouped into phrases
by testing for internal whitespace. Chinese, Japanese, Thai and the
other scripts written without inter-word spaces never satisfy that
test, so their phrase-level transcripts were treated as word-level and
re-grouped into a single cue covering the whole clip.

A three-phrase Chinese transcript produced one cue; the same transcript
in English produced three. The failure was silent: the export
succeeded, and the user found out by watching the captions.

For entries with no whitespace at all, fall back to entry length when
they are in a spaceless script. Whisper emits word-level tokens for
those scripts one or two characters at a time, while a phrase-level cue
runs to several times that. The median is used so one long token cannot
declare word-level input pre-grouped, and a couple of short cues cannot
declare a real transcript word-level.

--preserve-cues still forces the same thing, and behaviour for
space-separated scripts is unchanged.

Fixes heygen-com#3353

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at c16749da — COMMENT (non-blocking)

Verdict: APPROVE-shaped. The fix correctly solves #3353 and the approach — median entry length as the phrase-vs-word discriminator — is pragmatic and well-bounded. The Unicode ranges, threshold choice, and backward compatibility all check out. Three non-blocking observations below.

Correctness

Mechanism is sound. inferPreGrouped has two paths:

  1. Space-separated scripts (English, Korean, etc.): any entry with internal whitespace → pre-grouped. Unchanged from before, no regression.
  2. Spaceless scripts (CJK, Thai, Lao, Myanmar, Khmer): median entry length ≥ 4 chars → pre-grouped. This is the new path.

Unicode ranges verified. SPACELESS_SCRIPT_CHAR covers Thai (U+0E00–0E7F), Lao (U+0E80–0EFF), Myanmar (U+1000–109F), Khmer (U+1780–17FF), plus the existing CJK/kana ranges. All correct. Hangul exclusion is right — Korean uses inter-word spaces.

Median is the right central tendency here. some would false-positive on a single long token in word-level output; every would false-negative on a short greeting cue in phrase-level output. Median requires majority agreement, which matches the data shape.

Threshold of 4 is reasonable. Whisper word-level CJK tokens are typically 1–2 characters (occasionally 3 for compound readings). A real phrase-level cue is almost always ≥ 4 characters. The boundary case (3-char phrases like 你好吗) goes word-level, which is a tolerable false negative — those are rare in real transcripts and grouping them doesn't lose data, just splits display slightly differently.

Non-blocking observations

1. Mixed-script transcript edge case. If a transcript mixes English word-level tokens + one CJK phrase entry (e.g., bilingual narration), the CJK phrase's length can trigger inferPreGrouped → true, which then maps ALL entries (including the English ones) through entriesToCues as individual cues instead of grouping them. Example: [{text:"Hello"}, {text:"World"}, {text:"这是一个测试"}] → pre-grouped because median of spaceless = 6 ≥ 4 → three separate cues instead of "Hello World" + "这是一个测试". In practice this is unlikely (bilingual transcripts from whisper would have whitespace in the English phrases), but worth noting. A per-entry classification could handle this, but the added complexity isn't justified by the rarity.

2. Test coverage gap: exactly-at-threshold. The tests cover well above threshold (7-char phrases) and well below (1-char tokens), but not the boundary case: entries of exactly 4 characters (e.g., [{text:"你好世界"}, {text:"谢谢大家"}] — both 4 chars). Not blocking since the threshold logic is trivially correct, but a boundary test would document the design decision.

3. median on empty array. Returns 0, which is correct (0 < 4 → not pre-grouped). The guard if (spaceless.length === 0) return false makes this path unreachable anyway, so the defense-in-depth is fine as-is.

Backward compatibility

✅ The whitespace-based path runs first and short-circuits — any transcript that previously worked via whitespace detection still works identically.
✅ The existing "joins CJK word-level tokens without inserting spaces" test (1–2 char tokens) continues passing because median(1, 1, 2) = 1 < 4.
preGrouped: true override is unchanged.
✅ No API surface changes.

Clean fix. The new tests cover the critical cases (phrase-level CJK, phrase-level Thai, word-level CJK backward compat).

— Miga

@rajanpanth

Copy link
Copy Markdown
Contributor Author

Thanks, that is a genuinely useful review. Took the one concrete ask and verified the other.

Boundary test (observation 2). Added. Two entries of exactly four characters, which is the threshold, and it asserts they come back as two cues rather than one. It pins the constant rather than restating behaviour: moving SPACELESS_PHRASE_MIN_CHARS from 4 to 5 fails that test and nothing else in the file. 41 tests pass.

Mixed-script case (observation 1). Your analysis is exactly right, and I ran it rather than reasoning about it:

[{text:"Hello"}, {text:"World"}, {text:"这是一个测试"}]
  -> 3 cues: ["Hello", "World", "这是一个测试"]

The single CJK entry has a median length of 6, so the spaceless path fires and the English tokens get emitted individually instead of grouped. So it is real, not theoretical.

I have not fixed it, since you called the complexity unjustified and I agree at that rarity. If you ever want it closed, the cheap version is a majority guard rather than per-entry classification: only consult entry length when the spaceless entries are at least half of all entries, so a lone CJK phrase among English tokens cannot flip the whole transcript. That is one condition, not a restructure. Happy to add it here or leave it for a follow-up, your call.

Observation 3 matches what I intended: median([]) returning 0 is unreachable behind the spaceless.length === 0 guard, kept as defence in depth.

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Clean fix. Ship at HEAD 567dddfc.

Recap: wordsToCues inferred phrase-vs-word by internal-whitespace only, which is always false for CJK/Thai/Lao/Myanmar/Khmer, so phrase-level transcripts in those scripts collapsed into one cue spanning the whole clip. Fix adds a codepoint filter + median-entry-length ≥ 4 fallback for spaceless scripts, whitespace path unchanged. +45/-4 in normalize.ts, +47/0 in normalize.test.ts, all required CI green at HEAD.

Verified

Unicode rangesSPACELESS_SCRIPT_CHAR at packages/cli/src/whisper/normalize.ts:363 covers Thai U+0E00–0E7F, Lao U+0E80–0EFF, Myanmar U+1000–109F, Khmer U+1780–17FF, plus the CJK block (Symbols U+3000–303F, Hiragana+Katakana U+3040–30FF, Ext-A U+3400–4DBF, Unified U+4E00–9FFF, Compat U+F900–FAFF, Halfwidth/Fullwidth U+FF00–FFEF). Hangul correctly excluded — Korean uses inter-word spaces so the whitespace path handles it. CJK_CHAR at :357 untouched, so the existing join-without-separator rule is unchanged for Thai/Lao/Myanmar/Khmer (widening it is deferred to a separate PR — right call).
Median semantics (normalize.ts:370) — actual median, not mean; even-count returns average of two middle values (sorted[mid-1] ?? 0, sorted[mid] ?? 0); empty-array returns 0, defensively unreachable behind the spaceless.length === 0 guard on :395. Author's argument for median over some/every (one long token misclassifying word-level; a couple of short cues misclassifying phrase-level) holds.
Threshold-of-4 rationale — whisper CJK word-level tokens are 1–2 chars; phrase-level cues run several times that; 4 sits between. Boundary test at normalize.test.ts:320 ("treats entries at the length threshold as phrases", two 4-char entries) closes the exactly-at-threshold gap Miga flagged on r1 c16749da.
Backward compatopts.preGrouped ?? inferPreGrouped(...) at normalize.ts:414: explicit opt still wins; whitespace short-circuit runs first inside inferPreGrouped (:392), so every existing space-separated transcript takes the identical path as before. The existing "joins CJK word-level tokens without inserting spaces" test survives because median(1,1,1,2) = 1 < 4; the new test at normalize.test.ts:330 locks it in.
Reversion claim traced — stripping inferPreGrouped and reverting to words.some(/\s/) fails tests 1/2/3 at normalize.test.ts:295/:312/:320 (all whitespace-free CJK/Thai/4-char boundary → single collapsed cue) and passes test 4 at :330 (word-level tokens still one cue). Tests exercise the change, not restate old behavior.
inferPreGrouped shape — pure, non-exported, single caller (wordsToCues at :414), well-documented with JSDoc naming the design choice. Nothing else in the module changes.
CI at HEAD 567dddfc — 50 required check-runs green (Test, CLI smoke, Windows render/tests, Preview parity, Preflight, CodeQL js-ts + python, Producer unit + integration, SDK contract, etc). No skipped required.
Peer state — Miga posted a COMMENT (non-blocking, APPROVE-shaped) at r1 on c16749da flagging: mixed-script edge case, exactly-at-threshold test gap, empty-median defense. HEAD closes the threshold-test gap; empty-median defense confirmed sound; mixed-script noted below.

Non-blocking notes

Mixed-script edge case (confirms Miga's r1 note)Word[] = [{text:"Hello"}, {text:"World"}, {text:"这是一个测试"}] (English tokens per-word, no whitespace, one CJK phrase) → spaceless filter keeps only the CJK entry, median = 6 ≥ 4, inferPreGrouped returns true, all three become individual cues instead of joining "Hello World". Real whisper output almost never has this shape (bilingual whisper emits multi-word English phrases with internal whitespace, hits branch 1), but it's a genuine hole. Per-entry classification is the right shape and larger than this PR — fine to defer.
Test coverage — script variety — tests exercise Han and Thai. Hiragana/Katakana/Myanmar/Khmer are in the regex but not covered by an explicit case. Low risk (single regex, straightforward alternation), but one Japanese case (mixed Hiragana/Kanji) would document intent for the next reader.
String.length vs codepoint count (normalize.ts:397) — w.text.trim().length returns UTF-16 code units. All scripts in SPACELESS_SCRIPT_CHAR are BMP, so 1 code unit == 1 codepoint and the threshold behaves as written. If someone later extends the range to CJK Ext-B/C/D (U+20000+, surrogate pairs), the length would double-count and the threshold would drift. Not a defect today; worth a comment if that range is ever added.

What I didn't verify

• Didn't rebuild the CLI locally or run the test suite; I read the change and CI. Miga did the same. bunx vitest run packages/cli/src/whisper/ passing at 106 tests is the author's claim.
• Didn't run whisper end-to-end on a real Chinese/Thai audio clip; the fix is contained to wordsToCues shape-detection and the unit tests exercise both directions.

Merge is Miguel's call. I'm at 🟢 on my side.

Review by Rames D Jusso

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVED. Stamp basis: Miguel's explicit request in the HyperFrames channel. Rames D Jusso's pass is on record; what follows is my own verification.

I executed the classifier rather than reading it

Extracted SPACELESS_SCRIPT_CHAR, SPACELESS_PHRASE_MIN_CHARS and inferPreGrouped from normalize.ts at 567dddfc and ran them against real transcripts in each script:

input old (whitespace-only) new expected
JA phrase-level (#3353) false true (median 10) true
JA word-level false false (median 2) false
ZH phrase-level false true (median 6.5) true
ZH word-level false false (median 2) false
TH phrase-level false true (median 15.5) true
TH word-level false false (median 3.5) false
KO word-level false false (no spaceless entries) false
EN word / phrase false / true false / true unchanged
bare はい / いいえ false false (median 2.5) false
ZH word-level with 4-char idioms false false (median 2) false

11/11. The three ❌ rows are #3353 exactly, and nothing that used to work regressed.

A note on the Hangul exclusion, since it is the load-bearing claim in the comment: I first tested it with a retyped copy of the character class and got a false "Hangul matches" result. Re-deriving the class from the file and dumping it by code point settles it — the ranges are U+0E00‑0E7F, U+0E80‑0EFF, U+1000‑109F, U+1780‑17FF, U+3000‑303F, U+3040‑30FF, U+3400‑4DBF, U+4E00‑9FFF, U+F900‑FAFF, U+FF00‑FFEF, and Hangul syllables (U+AC00‑D7AF) and Jamo (U+1100) fall in none of them. Exclusion is real. Worth flagging for anyone reviewing Unicode classes by eye: don't retype them.

The design choices hold up

median over some/every is the right call and the reason is visible in the table — some would misclassify the 4-char-idiom row as pre-grouped, every would misclassify any phrase transcript containing one short cue. The even-length branch averages the two middles, the empty-array case returns 0 behind an already-unreachable guard, and an explicit opts.preGrouped still wins via ?? (including false, which is the case that matters).

Non-blocking: the threshold is much weaker for Thai than for CJK

The 4-char cutoff separates CJK cleanly because whisper emits 1–2 char tokens there. Thai words are genuinely long, so word-level Thai can clear the bar on its own:

["ความสุข"(7), "ประเทศไทย"(9), "มหาวิทยาลัย"(11)]  ->  median 9  ->  preGrouped = true

That is word-level input classified as phrase-level, so it would be emitted one cue per word instead of being regrouped. It is the mirror of the mixed-script edge already noted on this PR, and a different shape from it. I can't tell you how often real whisper Thai output looks like this — its Thai tokenization is subword-ish and may rarely produce three long standalone words — so I'm not treating it as a blocker, just as the known weak spot if Thai captions come back wrong. A per-script threshold (or median-of-character-count-per-second) is where I'd look first.

CI

48 success / 2 skipped at 567dddfc, with Detect changes cancelled. Not gating on it.

— Rames

@miga-heygen
miga-heygen merged commit 18409c9 into heygen-com:main Aug 26, 2026
50 of 51 checks passed
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.

CJK transcripts collapse into a single caption cue (wordsToCues whitespace heuristic)

4 participants