Skip to content

Keep each transcript segment on one TSV row - #2846

Open
codewithfourtix wants to merge 2 commits into
openai:mainfrom
codewithfourtix:fix-tsv-embedded-newlines
Open

codewithfourtix wants to merge 2 commits into
openai:mainfrom
codewithfourtix:fix-tsv-embedded-newlines

Conversation

@codewithfourtix

Copy link
Copy Markdown

Embedded newlines in segment text create extra rows in TSV output without timestamps. Replace carriage returns and newlines with spaces, as the writer already does for tabs.

Adds coverage for LF, CR, CRLF, and tab characters.

Copilot AI lite review requested due to automatic review settings September 5, 2026 17:21

Copilot AI 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.

🟢 Approval recommended

The change directly addresses the TSV row-splitting issue and includes focused regression tests for the relevant control-character cases.

Pull request overview

This PR hardens the TSV transcript writer so each segment is always emitted as a single TSV row by normalizing embedded control characters in segment text, preventing timestamp-less “extra rows” in downstream TSV consumers.

Changes:

  • Replace \r and \n in TSV segment text with spaces (in addition to existing tab handling).
  • Add pytest coverage for LF, CR, CRLF, and tab characters in segment text.
File summaries
File Description
whisper/utils.py Normalizes newline/carriage return characters in TSV output to keep segments on one row.
tests/test_tsv_writer.py Adds regression test ensuring segment text with LF/CR/CRLF/tab stays on a single TSV row.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread whisper/utils.py Resolved
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