Skip to content

fix(speaker-count): validate primitive type in normalizeStoredSpeakerCount - #1834

Closed
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1833-speaker-count-type-validation
Closed

fix(speaker-count): validate primitive type in normalizeStoredSpeakerCount#1834
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1833-speaker-count-type-validation

Conversation

@hsusul

@hsusul hsusul commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #1833

Problem

In src/helpers/speakerCount.js:
normalizeStoredSpeakerCount converted inputs directly with Number(value), causing boolean true to evaluate to 1 and array ["2"] to evaluate to 2 instead of rejecting non-numeric types as null.

Solution

  • Restricted normalizeStoredSpeakerCount to only numbers and strings (typeof value === "number" || typeof value === "string") before numeric conversion.
  • Added unit tests in test/helpers/speakerCount.test.js.

Verification

  • node --test test/helpers/speakerCount.test.js (passes, 3/3 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 Chadpiha self-assigned this Aug 25, 2026
@Chadpiha

Copy link
Copy Markdown
Collaborator

Thanks @hsusul — nice clean diff and the tests are in exactly our house style.

Closing this one together with #1833 after tracing where the stored value can actually come from: the cloud path is validated server-side with z.number().int().nullish() into a Postgres INTEGER column, the local path is a SQLite INTEGER read via better-sqlite3 (which can only yield numbers or null, and rejects binding booleans/arrays), and the renderer-side twin is typed number | null | undefined. So a boolean or array can't arrive at this helper in practice, and per our convention we keep type validation at those boundaries rather than re-checking downstream.

One thing worth knowing for the future: this helper has an intentional twin — normalizeSpeakerCount in src/services/uploadNotes.ts — and test/services/uploadNotes.test.js exists specifically to hold the two to identical outputs. Any behavior change to one needs the twin and the parity test updated in the same PR.

Really appreciate you digging into this area — thank you!

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.

normalizeStoredSpeakerCount accepts boolean true and array values as valid speaker counts

2 participants