diff --git a/backend/cpp/audio-cpp/stream_delta.cpp b/backend/cpp/audio-cpp/stream_delta.cpp index a328f7b90..97e1b3750 100644 --- a/backend/cpp/audio-cpp/stream_delta.cpp +++ b/backend/cpp/audio-cpp/stream_delta.cpp @@ -107,23 +107,49 @@ std::string TranscriptDeltaTracker::observe(const std::string &partial_text) { // delivered text would treat the held-back byte as new on the very next // report and emit it twice. const std::string known = assembled_ + pending_; - // Already accounted for. Covers the repeat voxtral produces when the sink - // and the return value carry the same event, and a hypothesis that shrank. + // An EXACT repeat, and nothing looser. This absorbs the duplicate delivery + // voxtral_realtime produces, which is the ONLY thing rule 2 was ever needed + // for: process_available_stream_chunks hands each event it produces to the + // sink from inside its loop and RETURNS the last of the batch + // (session.cpp:385-386), so that last event arrives twice with byte-equal + // text both times. A duplicate IS an exact repeat, so equality covers it. // - // The second condition is not decoration, and it is the whole of the second - // half of the UTF-8 bug. A fragment that ENDS MID-CHARACTER is not judged - // here at all, because the two readings of it are indistinguishable by - // bytes: "\xC3" is a prefix of an already-delivered "\xC3\x9F", and it is - // also the lead byte of a brand new "\xC3\xA9". Discarding it loses a - // character whose continuation bytes then arrive alone and begin the next - // delta with an orphan; holding it costs, at worst, a few duplicated bytes - // in a shrinking cumulative report, which no pinned family produces (every - // cumulative reporter here is append-only, and voxtral provably so, since - // its decode is a byte concatenation). One reading kills the RPC, the other - // glitches a hypothetical. Measured before this condition existed: - // 33.74% of Japanese traces carried at least one invalid delta. - if (starts_with(known, partial_text) && - utf8_complete_prefix_length(partial_text) == partial_text.size()) { + // It used to discard any report the known text merely STARTED WITH, and that + // cost far more than it bought. Two separate defects came out of it, and + // both were found by randomized traces rather than by reading: + // + // 1. A short INCREMENTAL fragment that happens to be a byte prefix of the + // transcript so far was read as a repeat and dropped, losing text with + // a 200 and no diagnostic. Both incremental families emit fragments + // that small routinely: nemotron_asr cuts at a byte offset + // (decoder.cpp:550) and vibevoice_asr at a common prefix + // (session.cpp:89-105). 9.50% of randomized pure-ASCII traces and + // 29.12% of French ones ended with a corrupted transcript. + // 2. When such a fragment was the LEAD BYTE of a multi-byte character, its + // continuation bytes then arrived alone and began the next delta, which + // is invalid UTF-8, which the Go runtime refuses to unmarshal, which + // ends the stream and the final_result with it. + // + // What the narrowing gives up is the shrinking-hypothesis case: a cumulative + // report SHORTER than what is known is now read as an incremental fragment + // and duplicates those bytes at the client. No pinned family produces one. + // voxtral is the only cumulative reporter, and its hypothesis is + // tokenizer_.decode(streaming_token_ids_) over a vector that is only ever + // push_back'ed (session.cpp:436) and cleared by reset() (session.cpp:257), + // so within a stream it can only grow. Measured: narrowing this changed not + // one byte of 30,000 randomized cumulative traces. + // + // KEPT DELIBERATELY THOUGH NO TEST CAN SEE IT. Once narrowed to equality + // this rule became redundant with rule 3 below: an equal partial has an + // empty suffix, so rule 3 would call release("") and emit nothing either + // way. Deleting it is therefore an equivalent mutation, and the mutation + // harness reports it as a survivor, which is the honest result and not a + // gap in the tests. It stays for two reasons: it states the duplicate + // absorption where the citation for it lives, and it is independent of rule + // 3's condition. A future tightening of rule 3 to, say, require a STRICTLY + // longer partial would otherwise send every duplicate down the incremental + // branch and put the whole transcript on the wire a second time. + if (partial_text == known) { return {}; } if (starts_with(partial_text, known)) { diff --git a/backend/cpp/audio-cpp/stream_delta.h b/backend/cpp/audio-cpp/stream_delta.h index 6fdfc3483..9becf49fb 100644 --- a/backend/cpp/audio-cpp/stream_delta.h +++ b/backend/cpp/audio-cpp/stream_delta.h @@ -69,13 +69,15 @@ public: // The rules, in order, all of them against everything KNOWN (delivered // plus held back), never against the delivered text alone: // 1. An empty partial says nothing. - // 2. A partial the known text ALREADY STARTS WITH, AND WHICH IS ITSELF A - // WHOLE NUMBER OF CHARACTERS, has been accounted for: nothing is - // emitted. This is what absorbs voxtral's repeat of the last event in - // each batch, and a cumulative hypothesis that shrinks. The second - // condition is what keeps a new character's lead byte from being - // mistaken for a repeat of an old character that starts with the same - // byte; see the note at the rule in the implementation. + // 2. A partial IDENTICAL to the known text is a repeat: nothing is + // emitted. Identical, not merely a prefix of it. That absorbs voxtral's + // repeat of the last event in each batch, which is the only duplicate + // any pinned family produces and which carries byte-equal text both + // times. Discarding a mere PREFIX used to swallow an incremental + // fragment that coincided with the start of the transcript, corrupting + // the text silently and, when that fragment was a character's lead + // byte, killing the stream outright; see the note at the rule in the + // implementation. // 3. A partial that EXTENDS the known text is a cumulative report: only // its new suffix is emitted. // 4. Anything else is an incremental fragment: it is emitted whole and diff --git a/backend/cpp/audio-cpp/stream_delta_test.cpp b/backend/cpp/audio-cpp/stream_delta_test.cpp index b769a2e16..1ceca4e2d 100644 --- a/backend/cpp/audio-cpp/stream_delta_test.cpp +++ b/backend/cpp/audio-cpp/stream_delta_test.cpp @@ -80,16 +80,37 @@ static void test_cumulative_family_with_duplicate_delivery() { check_eq(emitted.size() < 3 ? "" : emitted[2], " now", "third cumulative fragment"); } -// The rule that separates the two: text the client has ALREADY been sent is -// never sent again, whatever convention produced it. -static void test_already_delivered_text_is_never_resent() { +// The rule that separates the two: a report IDENTICAL to everything known is a +// repeat and is never sent again. +// +// Only an identical one. Rule 2 used to discard any report the known text merely +// STARTED WITH, and that cost more than it bought: see +// test_a_short_fragment_is_not_mistaken_for_a_repeat. +static void test_an_exact_repeat_is_never_resent() { TranscriptDeltaTracker tracker; check_eq(tracker.observe("hello world"), "hello world", "first fragment"); check_eq(tracker.observe("hello world"), "", "an identical repeat emits nothing"); - check_eq(tracker.observe("hello"), "", - "a shortened hypothesis emits nothing rather than duplicating a prefix"); - check_eq(tracker.assembled(), "hello world", - "a shortened hypothesis does not shrink what the client holds"); + check_eq(tracker.observe("hello world"), "", "and a third delivery emits nothing"); + check_eq(tracker.assembled(), "hello world", "the assembly is unchanged by repeats"); +} + +// THE TRADE, pinned so it is a decision rather than a surprise. A CUMULATIVE +// report that SHRINKS is no longer absorbed: it is read as an incremental +// fragment and duplicates a few bytes at the client. +// +// No pinned family produces one. voxtral_realtime is the only cumulative +// reporter, and its hypothesis is tokenizer_.decode(streaming_token_ids_) over a +// vector that is only ever push_back'ed (session.cpp:436) and cleared by reset() +// (session.cpp:257), so within a stream it grows and never shrinks. The +// duplicate delivery rule 2 really exists for is an EXACT repeat, which the test +// above still covers. +static void test_a_shrinking_hypothesis_is_read_as_incremental() { + TranscriptDeltaTracker tracker; + check_eq(tracker.observe("hello world"), "hello world", "first fragment"); + check_eq(tracker.observe("hello"), "hello", + "a shortened hypothesis is now read as an incremental fragment"); + check_eq(tracker.assembled(), "hello worldhello", + "which duplicates those bytes at the client: the accepted cost"); } static void test_empty_partials_are_ignored() { @@ -407,10 +428,39 @@ static void test_a_fragment_never_begins_mid_character() { check_eq(all_orphans.observe("after"), "after", "the stream continues"); } +// PURE ASCII, no multi-byte character anywhere, and the transcript still comes +// out wrong: a short incremental fragment that happens to be a byte prefix of +// everything known was read as an already-delivered repeat and discarded. +// +// This is the shape both incremental families produce. nemotron_asr emits +// current_text.substr(emitted_text.size()) per non-blank token (decoder.cpp:550) +// and vibevoice_asr emits text.substr(common_prefix_size(...)) (session.cpp:89-105), +// so a one-character fragment is ordinary output, and any of the transcript's +// own leading characters will eventually arrive as one. +// +// Measured over 5,000 randomized traces per transcript before this was fixed: +// 9.50% of pure-ASCII traces and 29.12% of French ones ended with the client +// holding something other than final_result.text, with a 200 and no diagnostic. +static void test_a_short_fragment_is_not_mistaken_for_a_repeat() { + TranscriptDeltaTracker tracker; + std::vector emitted; + const std::string full = "pure ascii transcript"; + const std::string view = client_view( + tracker, {"pure ", "ascii ", "trans", "c", "ri", "p", "t"}, &emitted); + + check_eq(view, full, "the whole transcript reaches the client"); + check_eq(tracker.assembled(), full, "and the assembly agrees with it"); + check_eq(tracker.reconcile(full), "", + "the final text adds nothing, because nothing was lost"); + check(emitted.size() == 7, "every fragment produced exactly one delta"); +} + int main() { test_incremental_family(); test_cumulative_family_with_duplicate_delivery(); - test_already_delivered_text_is_never_resent(); + test_an_exact_repeat_is_never_resent(); + test_a_shrinking_hypothesis_is_read_as_incremental(); + test_a_short_fragment_is_not_mistaken_for_a_repeat(); test_empty_partials_are_ignored(); test_offline_fallback_is_one_delta(); test_reconcile_emits_the_tail();