Skip to content

fix(srt): handle undefined/null text in malformed SRT files#1990

Merged
cherkanovart merged 4 commits intomainfrom
feat/srt-loader-improvements
Feb 17, 2026
Merged

fix(srt): handle undefined/null text in malformed SRT files#1990
cherkanovart merged 4 commits intomainfrom
feat/srt-loader-improvements

Conversation

@cherkanovart
Copy link
Contributor

@cherkanovart cherkanovart commented Feb 17, 2026

Problem

SRT parser can return entries with undefined text when encountering malformed SRT files (bad timestamps, missing separators). This causes TypeError: Cannot read properties of undefined (reading 'replace') when trying to serialize the SRT back.

Example malformed entries:

  • 00:01:44,480 --> 00:0,960 (invalid timestamp)
  • 00:02:30,400 --00:02:35,240 (missing > separator)

Solution

Filter undefined/null entries in both pull() and push():

  • pull(): Skip malformed source entries early (prevents undefined from entering translation pipeline)
  • push(): Silent filter (prevents crash, entries get filled from sourceData in final merge)

Why No Warnings?

Initial implementation had warnings, but they were misleading:

  • Entries are filtered during streaming chunks
  • But final merge with sourceData fills them back in
  • Final output is complete, so warnings caused unnecessary concern

Changes

  • Added filter in pull() to skip entries with undefined/null text
  • Added filter in push() to prevent crash
  • Added tests for both scenarios (including verification that malformed entries are filtered)
  • No warnings (silent filtering since final output is correct)

Testing

pnpm test -- src/cli/loaders/index.spec.ts -t "srt"

All tests pass ✅

User Feedback

Tested by users with long SRT files - works correctly, no missing entries in final output.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

  • Fixed an issue in the SRT subtitle system where undefined or null subtitle entries would cause problems during the push operation. These invalid entries are now properly filtered out.

Warnings were misleading since entries get filled from sourceData in final merge.
Final output is correct, so silent filtering is appropriate.
Resolved conflict: keeping version without warnings (they're misleading)
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 17, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

A patch release for the SRT subsystem that simplifies subtitle entry filtering by removing undefined or null values during push operations. The console warning previously logged for missing text entries has been eliminated, relying solely on truthiness checks for filtering.

Changes

Cohort / File(s) Summary
Changeset documentation
.changeset/fix-srt-undefined.md
New changeset entry documenting the patch fix for undefined/null SRT subtitle filtering in the push operation.
SRT loader
packages/cli/src/cli/loaders/srt.ts
Simplified push-side filter logic by removing the console.warn side effect when filtering undefined or null subtitle entries, now using direct truthiness checks only.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • AndreyHirsa

Poem

🐰 A hop through code so neat,
Undefined entries deleted, the fix complete—
No warnings now, just filters true,
SRT subtitles shine brand new! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 accurately describes the main change: handling undefined/null text in malformed SRT files, which matches the core fix in the pull request.
Description check ✅ Passed The description covers the problem, solution, changes, and testing sections. However, it lacks explicit Business Logic Tests checklists and Before/After visuals sections from the template.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/srt-loader-improvements

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

@cherkanovart cherkanovart merged commit aa4033c into main Feb 17, 2026
10 checks passed
@cherkanovart cherkanovart deleted the feat/srt-loader-improvements branch February 17, 2026 16:24
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