Skip to content

fix(cli): preserve speaker labels in WebVTT cue text - #5233

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/cli-vtt-speaker-text
Oct 8, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/cli-vtt-speaker-text

Conversation

@user-github-me

Copy link
Copy Markdown
Contributor

Importing a valid WebVTT cue containing ALICE: Hello there silently drops that cue. The parser removes uppercase colon-prefixed lines throughout the file as metadata, so it also truncates multiline captions and changes later cue IDs.

Remove the global metadata-line deletion. Header metadata already sits outside timed cue payloads and is ignored by the existing block parser. Add speaker-label and multiline regressions with an X-TIMESTAMP-MAP header, plus SRT/VTT round trips, preserving cue text, timing, and IDs.

Validation:

  • Five new regressions fail before the fix; 220 caption/transcription tests pass afterward, with one existing platform skip.
  • Actual built CLI reproduces the missing cue before the fix. Afterward, six imports preserve both cues; twelve repeated SRT/VTT exports remain stable and leave the source bytes unchanged.
  • Chrome native TextTrack parsing confirms expected text and timestamps for all twelve original/exported WebVTT cases.
  • Rebased on current main 188475aaf; all package builds and the 220 related tests passed again. The rebuilt CLI reproduction, CLI typecheck, repository lint, formatting, comment checks, and all commit hooks passed on that base.

@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.

The uppercase-colon metadata strip runs over the whole file, so cue lines like ALICE: Hello are deleted and later cue ids shift. Dropping it is safe: header lines either sit in a block with no timing line or come before the --> line, and only lines after the timing line become cue text. The tests fail on main and pass here. This conflicts textually with #5223 on the same line, so whichever lands second should keep both the CRLF normalization and this removal.

— Rames

@jrusso1020
jrusso1020 enabled auto-merge October 8, 2026 19:34
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 8, 2026
Merged via the queue into heygen-com:main with commit 7faff81 Oct 8, 2026
80 of 81 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.

2 participants