WhisperKit : drop negative sentinels from SuppressTokensFilter (#392)#460
Open
achyutbenz19 wants to merge 1 commit intoargmaxinc:mainfrom
Open
WhisperKit : drop negative sentinels from SuppressTokensFilter (#392)#460achyutbenz19 wants to merge 1 commit intoargmaxinc:mainfrom
achyutbenz19 wants to merge 1 commit intoargmaxinc:mainfrom
Conversation
DecodingOptions.supressTokens defaults to [-1] in the OpenAI convention
("-1 means suppress all non-speech tokens"), but WhisperKit never
expands the sentinel. The filter at TextDecoder.createLogitsFilters
passed -1 through to SuppressTokensFilter, whose init built the index
[0, 0, -1] and called MLMultiArray.fill, which reached into dataPointer
at a negative offset. On iOS 26 (where CoreML started returning
read-only MLMultiArrays) the negative write landed on a protected page
and crashed with EXC_BAD_ACCESS / SIGBUS.
Filter to [0, specialTokenBegin) at the call site and, as defence in
depth, filter negatives in SuppressTokensFilter.init. Add a unit test
covering mixed positive / negative suppressTokens input.
Fixes argmaxinc#392
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #392.
DecodingOptions.supressTokensdefaults to[-1]using the OpenAI whisper convention, where-1is a sentinel meaning "suppress every non-speech special token." WhisperKit never expands the sentinel, so the value flows throughcreateLogitsFiltersintoSuppressTokensFilter, whoseinitbuilds the index array[[0, 0, -1]]. WhenfilterLogitscallslogits.fill(indexes:with:), the helper computeslinearOffset = -1 * strides[2]and writes-FloatType.infinityatdataPointer - strides[2]. On iOS 26 (where CoreML began returning read-onlyMLMultiArrayfor performance), that negative write lands on a protected page and the process crashes withEXC_BAD_ACCESS (SIGBUS) / KERN_PROTECTION_FAILURE.Scope of the change
Two one-line filter predicates plus one unit test.
Sources/WhisperKit/Core/TextDecoder.swift,createLogitsFilters: change the pre-filter that was{ $0 < specialTokenBegin }to{ $0 >= 0 && $0 < specialTokenBegin }.Sources/WhisperKit/Core/Text/LogitsFilter.swift,SuppressTokensFilter.init: filter negatives out ofsuppressTokensbefore buildingsuppressTokenIndexes. Defence in depth so that any caller building a filter with a negative id also gets a safe no-op.Tests/WhisperKitTests/UnitTests.swift,testSuppressTokensFilter: add a case with[-1, 0, -5, 3]and assert the logits at positions 0 and 3 are suppressed while negatives are ignored.Reproduction
Prior to the patch, running the existing test suite with the new assertion produces a silent memory corruption (the negative write lands inside the test's own
MLMultiArrayallocation, shifting by-strides[2]bytes). On an iOS 26 device with a real transcription run, the same codepath segfaults as described by the reporter.After the patch, the new test case passes:
The existing three assertions in
testSuppressTokensFiltercontinue to pass unchanged, confirming the positive-id path is not affected.Differential matrix
This is a pure Swift library change with deterministic output; the regression signal is the unit test. I did not run
audiokit regress checkon this one because the fix has no effect when everysuppressTokensvalue is already in[0, specialTokenBegin)(which is the case for all real-world WAV fixtures the matrix would exercise). The audit surface here is the invalid-id path, which is now covered by the test case.What this does not do
MLMultiArrayissue. If a caller buildsSuppressTokensFilterwith only valid positive ids on iOS 26, the in-placefillstill lands on a read-only page. That is a separate, bigger problem (affects everyLogitsFilterthat mutates logits in place) and is best handled with a comprehensive switch to copy-on-modify, which is out of scope here.-1sentinel should mean. OpenAI whisper expands it to the set of non-speech tokens. If that expansion belongs somewhere, it can be added separately; for now, ignoring the sentinel matches the "do not crash" contract without changing which tokens get suppressed for any non-default configuration.Tools used
git,swift build,swift test, andaudiokiton other PRs in this series. No audio fixtures needed for this one.Disclosure
I am an AI assistant (Anthropic's Claude) helping a user contribute this fix. I have not personally reproduced the crash on an iOS 26 device (I worked on macOS), but the code path is clear by inspection and the new unit test demonstrates the negative-id behavior is now safe.