Skip to content

fix(speech): removing filler words no longer joins paragraphs - #34101

Open
EtanHey wants to merge 1 commit into
pingdotgg:mainfrom
EtanHey:fix/filler-removal-keeps-paragraphs
Open

EtanHey wants to merge 1 commit into
pingdotgg:mainfrom
EtanHey:fix/filler-removal-keeps-paragraphs

Conversation

@EtanHey

@EtanHey EtanHey commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Problem

With Remove filler words enabled, First paragraph.\n\nSecond paragraph. becomes First paragraph. Second paragraph. even though it contains no filler words. The shared whitespace cleanup joins paragraph breaks in cloud transcription, or any transcript that contains line breaks.

Change

Collapse repeated whitespace excluding CR and LF, remove whitespace around those line breaks, and retain the existing outer trim. Single-line cleanup remains unchanged. Extend the existing batch/stream test table with paragraph preservation and fillers at LF/CRLF line edges.

Scope and approval

No issue: small fix for an obvious bug, because the setting is about filler words and the paragraph loss happens even when there are none.

Verification

  • Base: origin/main 17a6a01d84e3adf9f29df2639d6b6079a434e1b2, with regression rows added before the source fix.
  • vp test run apps/server/src/speech/SpeechService.test.ts: before fix, 3 failed / 37 passed (40). Expected First paragraph.\n\nSecond paragraph., received First paragraph. Second paragraph.; the filler and LF/CRLF edge cases likewise lost breaks.
  • Same command after fix: 40 passed (40), exercising batch and stream finalization.
  • vp run check:changed: passed format, lint, GraphQL, typecheck (3 packages), knip, and tests (42 passed across 2 files). Full CI suites and build were not run.
  • Direct Node calls to the actual function preserved LF/CRLF paragraphs and removed English fillers. Both callers (CloudSpeech.ts, SpeechService.ts) return strings with only outer trim after filler cleanup; no single-line dependency there.
  • No live speech provider, native audio, or client UI verification was run.

Prior PRs

#32406 changes optional punctuation matching after a filler to consume ellipses. This fix leaves that pattern untouched and changes only subsequent whitespace cleanup. They compose semantically: an in-memory combination of the two changes produced First paragraph.\n\nSecond. from Um... First paragraph.\n\nUh… Second.. Nearby edits in the same function/test table may need a trivial textual rebase; neither change supersedes the other.

Review: one other-vendor round (Claude Opus 5.5): code approved; two redundant test rows removed and wording narrowed to transcripts that contain line breaks.

🤖 Implemented by GPT-6.1 Sol (Codex) in T3 Code; reviewed by Claude Opus 5.5 (Claude Code).

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
@EtanHey
EtanHey marked this pull request as ready for review November 10, 2026 23:10
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 10, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4a1f455

Macroscope's review found this PR approvable — Speech filler cleanup now preserves paragraph breaks instead of joining paragraphs. The change is confined to transcript whitespace formatting, with no settings-default or external-system changes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

removeSpeechFillerWords now normalizes horizontal whitespace without collapsing line breaks. Tests cover removing “um” and “uh” across paragraph breaks while preserving blank lines and LF or CRLF endings.

Changes: Speech filler whitespace preservation

Layer / File(s) Summary
Preserve line breaks during filler removal
apps/server/src/speech/fillerWords.ts, apps/server/src/speech/SpeechService.test.ts
The filler-removal function normalizes horizontal whitespace, removes horizontal whitespace around line breaks, and trims the result. Tests verify filler removal across paragraph breaks while preserving blank lines and line endings.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Possibly related PRs

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 4a1f4

No merge-blocking risk was identified in the paragraph-preservation change.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly summarizes the main change: filler-word cleanup no longer joins paragraphs.
Description check Passed The description covers the problem, change, scope rationale, and focused verification results. It also states relevant checks that were not run.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 17a6a01 and 4a1f455.

Files selected for processing: 2

  • apps/server/src/speech/SpeechService.test.ts
  • apps/server/src/speech/fillerWords.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@maria-rcks

maria-rcks commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

partial verification at 4a1f4557910d23aaa6b461a36b938304730794e8 on linux. the paragraph fix is not verified end to end.

  • installed and ran github/main (b883156e26) in a separate worktree, then stopped it before running the exact pr head. used playwright with system chrome against the actual dev web app.
  • created a project for a real git repo through the app. real codex gpt-6.1-sol turns completed on main and the pr head.
  • downloaded parakeet unified english through voice settings. synthesized spoken audio passed through the operating-system audio input, web composer, voice websocket and real local model. finishing dictation removed um and uh; cancel preserved the existing draft. ai cleanup was disabled. no transcription responses were mocked.
  • the running server's authenticated transcription endpoint returned the same single-line result on main and the pr. filler removal off retained both fillers; on removed them. on the pr, correction cue um stayed intact and a dictionary alias still applied.
  • blacksmith: vp test run apps/server/src/speech/SpeechService.test.ts apps/server/src/speech/CloudSpeech.test.ts passed all 44 tests across both files.

pr head: final composer dictation with fillers removed

pr head: live dictation, finish, and cancel preserving the draft

runtime command transcript

Not verified

provider-returned paragraph breaks through the real client and speech provider. all three cloud speech keys were unconfigured. local parakeet transcribed the spoken words "new paragraph" literally, so neither main nor the pr received a multiline transcript. direct calls reproduced main collapsing LF paragraphs and the pr preserving LF/CRLF, but those calls and the passing tests do not establish the requested end-to-end proof.

native desktop, ios and android clients, other speech models, and real cloud speech services were not exercised. no code was changed or pushed. completing paragraph verification needs a real cloud speech credential and audio that returns multiline transcription.

@EtanHey

EtanHey commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Verified: provider-returned paragraph breaks

ran an authenticated HTTP A/B on macOS with a real local OpenAI-compatible transcription server and #32530's custom speech path. the server transcribed an archived clip; no responses were mocked. no audio was played or recorded.

A was 132e5c5b279e96045c45ff07465a432172f9e1dd. B was A plus 4a1f4557910d23aaa6b461a36b938304730794e8 (combined head c7c6a2a20c0863e0febd0fa23cea1a5cfaacc6a3). filler removal was enabled in both. the same PCM request was used, and the provider's retained WAV hashes matched the direct input.

  • direct provider: HTTP 200, 3 lines / 2 paragraphs; text SHA256 ddae71dfa2371117b2e6713d8c552ce50f023173a9378df2b0af7cd4a0122f54.
  • A: HTTP 200, 1 line / 1 paragraph; text SHA256 93f73245e14d7f0d74d8d7acafbf8444f060307a701214907684bde551adadcd.
  • B: HTTP 200, 3 lines / 2 paragraphs; text SHA256 ddae71dfa2371117b2e6713d8c552ce50f023173a9378df2b0af7cd4a0122f54.

B matched the provider text exactly after trimming leading/trailing whitespace. A differed only by replacing the blank line with one space. hashes are of trimmed UTF-8 text. all test processes were stopped.

Not verified

rendered web/composer behavior, native desktop, ios or android clients, and cloud vendors were not exercised. this verifies the real authenticated server/provider path, not the remaining rendered-client paragraph check. no code was pushed and no installed app was changed.

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

The old \s{2,} → " " collapse was what joined paragraphs. Collapsing only horizontal whitespace ([^\S\r\n]{2,}) and then trimming horizontal whitespace around each CR/LF fixes it while keeping single-line output the same. The new table rows run through both batch and stream finalization and cover LF, CRLF, and fillers at line edges. The external A/B in the comments also confirms the provider text now survives intact.

No defects found. Two small behaviour notes, neither needs a change:

  1. (low) apps/server/src/speech/fillerWords.ts:42-43 — runs of blank lines are no longer collapsed (before, \n\n\n became one space; now it stays three newlines), and leading indentation on each line is stripped. Both look fine for dictation output; mentioning them only because this function is shared by CloudSpeech.ts and SpeechService.ts.
  2. (low) Unicode line/paragraph separators (U+2028/U+2029) still count as horizontal whitespace under [^\S\r\n], so they are still folded to a space. Unlikely from current providers; adding \u2028\u2029 to both character classes would make the rule "preserve any line break" exact.

Agree with the note about #32406 that the two changes compose; a rebase there should be textual only.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants