Skip to content

Transcription temperature: malformed values are silently reinterpreted instead of rejected #4552

Description

@Daksha1611

Describe the bug

/v1/audio/transcriptions silently reinterprets a malformed temperature field instead of rejecting it, because a second, looser parser rescues values the first one correctly refused.

src/audio/speech_to_text/s2t_servable.cpp:75-90:

auto temp = ovms::stof(temperatureStr);
if (!temp.has_value()) {
    temp = ovms::stou32(temperatureStr);
    if (!temp.has_value())
        return absl::InvalidArgumentError("Invalid temperature type.");
}
config.temperature = temp.value();
if (config.temperature != 0) {
    config.do_sample = true;
}

ovms::stof (src/stringutils.cpp:196-216) requires the whole string to be consumed and rejects NaN/Inf. ovms::stou32 (src/stringutils.cpp:112-129) does neither — it is the only converter in stringutils that does not check idx against the string length, and it calls erase_spaces() on its input first.

So the fallback accepts exactly the values stof had just rejected:

field parsed as correct result
temperature=0.5x 0.0 — not 0.5 400
temperature=0abc 0.0 400
temperature=1e400 1.0 (stof throws out_of_range, stou32 parses the leading 1) 400
temperature=5 5 55.0 (spaces erased first) 400

Verified by compiling both versions of the function with ovms::stof, ovms::stou32 and erase_spaces copied verbatim:

  0.5x        before: accepted t=0 do_sample=false        after: 400 Invalid temperature type.
  0abc        before: accepted t=0 do_sample=false        after: 400 Invalid temperature type.
  1e400       before: accepted t=1 do_sample=true         after: 400 Invalid temperature type.
  5 5         before: accepted t=55 do_sample=true        after: 400 Invalid temperature type.
  abc         before: 400 Invalid temperature type.       after: 400 Invalid temperature type.

The 0.5x case is the most user-visible: a typo in a value that looks almost right silently turns sampling off and transcribes greedily, with a 200 response and no warning.

The fallback also appears to serve no purpose. ovms::stof already parses integer spellings ("1" → 1.0f), so an integer-only parser is never needed as a second chance — every input it can rescue is malformed by definition.

Not reporting negative temperature as a bug. temperature=-1 is accepted and enables sampling, but that is deliberate: #4201 ("Negative temperature should lead to genai error in STT path") established that such values are passed through for GenAI to reject, and negativeTemperatureEnableSampling in src/test/audio/speech2text_test.cpp pins it. Noting it here only so it is clear it was considered and intentionally left alone.

To Reproduce

curl http://localhost:8000/v1/audio/transcriptions \
  -F file=@audio.wav -F model=whisper-tiny -F temperature=0.5x
# 200 OK, transcribed with temperature 0.0 (sampling disabled) - expected 400

curl http://localhost:8000/v1/audio/transcriptions \
  -F file=@audio.wav -F model=whisper-tiny -F temperature=1e400
# 200 OK, transcribed with temperature 1.0 - expected 400

Expected behavior

A temperature field that is not entirely a number returns 400 Invalid temperature type. — which is what the code already intends; the fallback just prevents it from firing.

Logs

At --log_level TRACE, Received temperature: 0.5x is logged, followed by a normal transcription.

Configuration

  1. OVMS version: main @ fadb3314
  2. Any speech-to-text servable (e.g. whisper-tiny)
  3. CPU
  4. N/A
  5. Any audio file

Additional context

Suggested fix: drop the ovms::stou32 fallback so the existing Invalid temperature type. error path fires. The three existing SttServableParseTemperatureTest cases (-1.0, 0, 1.0) all parse cleanly through ovms::stof and never reached the fallback, so they are unaffected.

Related: the root cause is that ovms::stou32 does not verify the whole string was consumed, unlike stou64, stoi32 and stof in the same file. That is filed separately as a stringutils inconsistency, since it also affects the GRPC_SERVERS environment variable. Fixing either one independently fixes this endpoint; the two do not conflict.

I have a patch for this and will open a PR shortly.

Activity

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

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions