Skip to content

fix(diarization): guard nullish inputs and format non-finite timestamps in speakerMerge - #1780

Closed
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1779-speaker-merge-null-safety
Closed

fix(diarization): guard nullish inputs and format non-finite timestamps in speakerMerge#1780
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1779-speaker-merge-null-safety

Conversation

@hsusul

@hsusul hsusul commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #1779

Problem

In src/helpers/speakerMerge.js:

  1. splitIntoSentences(text) passed text directly to Intl.Segmenter, which coerces null or undefined into string values ("null" / "undefined"), causing splitIntoSentences(null) to return ["null"]. In environments without Intl.Segmenter, calling splitIntoSentences(null) threw a TypeError.
  2. formatTimestamp(seconds) did not sanitize non-number, non-finite, or negative inputs, returning "NaN:NaN" on undefined/NaN/Infinity or negative minute/second components like "-1:-5" on negative numbers.
  3. formatSpeakerTranscript(mergedSegments) threw TypeError: Cannot read properties of null (reading 'map') when mergedSegments was nullish.
  4. mergeSpeakersWithText(segments, text) produced segments with text: null when text was null.

Solution

  • Added explicit string and empty-string guards at the top of splitIntoSentences.
  • Sanitized seconds in formatTimestamp so that non-finite, sub-zero, or nullish inputs safely default to 0 seconds ("0:00").
  • Added array guard in formatSpeakerTranscript to safely return "" for nullish or empty inputs.
  • Sanitized input text in mergeSpeakersWithText to default to "".
  • Added comprehensive regression tests in test/helpers/speakerMerge.test.js.

Verification

  • node --test test/helpers/speakerMerge.test.js (passes, 17/17 tests)
  • npm run typecheck (passes, 0 errors)
  • npm run lint (passes, 0 errors)
  • npm run i18n:check (passes)
  • npm run build:renderer (passes)
  • git diff --check (clean)

@Chadpiha

Copy link
Copy Markdown
Collaborator

Thanks @hsusul! Closing with #1779 — this is a case where the validation you're after already exists, one layer up at the real boundary.

The merge-speaker-text IPC handler validates everything before these helpers run: it rejects non-array segments, non-string text, and non-finite durations, caps sizes, and re-sanitizes each segment (speaker → sliced string or "unknown", start/end → finite number or 0). formatTimestamp's only other caller feeds it internal constants that are Number.isFinite-guarded at the call site. So the helpers can only ever see sanitized input, and duplicating the checks one layer deeper is the pattern our conventions steer away from (see the note on #1840).

Thanks for the attention to the diarization path! 🙏

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.

splitIntoSentences and formatTimestamp in speakerMerge produce invalid output on nullish or non-finite inputs

2 participants