Skip to content

fix(cli): vtt style region blocks#2057

Merged
AndreyHirsa merged 4 commits intomainfrom
fix/vtt-style-region-blocks
Mar 26, 2026
Merged

fix(cli): vtt style region blocks#2057
AndreyHirsa merged 4 commits intomainfrom
fix/vtt-style-region-blocks

Conversation

@AndreyHirsa
Copy link
Copy Markdown
Contributor

@AndreyHirsa AndreyHirsa commented Mar 26, 2026

Summary

Fix VTT parser crash when files contain STYLE or REGION blocks, which are valid WebVTT features unsupported by the node-webvtt library.

Changes

  • Extract STYLE/REGION blocks before passing VTT content to node-webvtt parser
  • Re-insert STYLE/REGION blocks into translated output after the WEBVTT header

Testing

Business logic tests added:

  • STYLE block is stripped during pull and cues are parsed correctly
  • STYLE block is preserved in push output alongside translated cues
  • REGION block is stripped during pull and cues are parsed correctly
  • REGION block is preserved in push output alongside translated cues
  • All tests pass locally

Visuals

N/A — no UI changes.

Checklist

  • Changeset added (if version bump needed)
  • Tests cover business logic (not just happy path)
  • No breaking changes (or documented below)

Summary by CodeRabbit

  • Bug Fixes

    • Fixed a VTT parser crash for files containing STYLE or REGION blocks; non-cue blocks are now handled and preserved through import/export.
  • Tests

    • Added tests verifying pull ignores non-cue blocks and push preserves STYLE/REGION content while updating cue text.
  • Chores

    • Added a changeset recording a patch release for the VTT fix.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2f965e6f-d25d-4c5b-b35d-260950d49f2a

📥 Commits

Reviewing files that changed from the base of the PR and between a7ebd61 and 48495d7.

📒 Files selected for processing (1)
  • packages/cli/src/cli/loaders/vtt.ts

📝 Walkthrough

Walkthrough

Added a changeset and updated the VTT loader to strip STYLE/REGION blocks during pull() and preserve/reinsert them after the WEBVTT header during push() (push signature now accepts an originalInput third parameter). Tests added for extraction and preservation behavior.

Changes

Cohort / File(s) Summary
Changeset
.changeset/violet-pens-build.md
New patch changeset noting a fix for a VTT parser crash when STYLE/REGION blocks are present.
VTT Loader Implementation
packages/cli/src/cli/loaders/vtt.ts
Added helpers to detect and extract STYLE/REGION blocks, normalize line endings, and split by blank-line blocks. pull() strips unsupported blocks before parsing; push(locale, payload, originalInput) can reinsert original STYLE/REGION blocks immediately after the WEBVTT header. (Push signature updated.)
VTT Loader Tests
packages/cli/src/cli/loaders/index.spec.ts
Added four tests verifying pull() ignores STYLE/REGION (returns only cue entries) and push() preserves original STYLE/REGION blocks while applying updated cue texts.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • vrcprl

Poem

🐇 I found the VTT with regions and style,
They tangled the parser and lingered awhile.
I nibbled them out, kept each cue in its place,
Then stitched them back neat, right after the base.
Hooray — the captions now hop with grace!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(cli): vtt style region blocks' is concise and directly related to the main change: fixing VTT parser crashes caused by STYLE/REGION blocks.
Description check ✅ Passed The PR description matches the template structure with complete Summary, Changes, Testing, Visuals, and Checklist sections. All required business logic tests are documented with checkboxes marked.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/vtt-style-region-blocks

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

@AndreyHirsa AndreyHirsa changed the title Fix/vtt style region blocks fix(cli): vtt style region blocks Mar 26, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/cli/src/cli/loaders/vtt.ts`:
- Around line 33-42: The loader currently uses a single module-level savedBlocks
array that is overwritten in pull(), causing cross-locale leakage; update the
implementation in createLoader's pull/push logic to keep extracted blocks
per-locale (keyed by the locale argument) or re-derive blocks from the
locale-specific pullInput available in push(), using
extractUnsupportedBlocks(input) for that locale instead of the shared
savedBlocks; update both places that read/write savedBlocks (the pull()
assignment where savedBlocks = blocks and the later code that consumes
savedBlocks) to use the per-locale map or to compute blocks from the pullInput
passed to push().
- Around line 14-19: The unsupportedRegex currently matches prefixes like
"STYLE" or "REGION" and thus incorrectly strips cues such as "REGION42" or
"STYLE_intro"; update the check in the loop that examines firstLine by trimming
trailing whitespace (use trim() or trimEnd()) and change unsupportedRegex to
anchor the end of the line (e.g., /^(?:STYLE|REGION)\s*$/) so it only matches
exact header lines; adjust the test in the for-loop that uses
unsupportedRegex.test(firstLine) accordingly (see unsupportedRegex and the loop
over parts / firstLine).
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 05ec2afc-11d8-40b6-a0f6-b6a241c7f463

📥 Commits

Reviewing files that changed from the base of the PR and between d18be81 and bb15da3.

📒 Files selected for processing (3)
  • .changeset/violet-pens-build.md
  • packages/cli/src/cli/loaders/index.spec.ts
  • packages/cli/src/cli/loaders/vtt.ts

@AndreyHirsa AndreyHirsa merged commit abe3d2f into main Mar 26, 2026
9 checks passed
@AndreyHirsa AndreyHirsa deleted the fix/vtt-style-region-blocks branch March 26, 2026 10:47
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