testing qvac-lib-dl-hyperdrive ci#11
Merged
Merged
Conversation
Contributor
|
Requesting review from: @ignaciolarranaga [auto_pr_review_request] |
ishanvohra2
pushed a commit
that referenced
this pull request
Apr 24, 2026
Polish the remaining review nits on the TTS client streaming surface. - #3 TtsMulticast.pump now rejects the `done` promise with the fatal error instead of resolving `false`. An internal `.catch(() => {})` silences unhandled-rejection warnings when the caller only iterates the buffer/chunk streams and never awaits `done`; re-awaits still see the rejection. - #6 TextToSpeechStreamSession[Symbol.asyncIterator] no longer throws synchronously on a second iteration; it returns an iterator whose first `.next()` rejects, so `for await` surfaces the error in the normal async control flow rather than the iterator protocol. - #9 plainTtsBufferStream / collectTtsBuffer wrap the RPC loop in try/catch/finally so `done` always settles: resolve(true) on the terminal frame, reject with the real error on exceptions, and resolve(false) on early consumer break. Previously `await done` could hang forever when the consumer bailed out early. - #11 Skip per-frame ttsResponseSchema.parse() in all three paths; rely on the discriminated-union narrowing at the RPC boundary. Drops the per-PCM-frame Zod validation cost for large sentences. Made-with: Cursor
ishanvohra2
added a commit
that referenced
this pull request
Apr 27, 2026
…peech (#1590) * feat: Add runStream() which takes input as a stream * add integration tests * uncomment cb tests * chore: Add cb streaming example * feat: Add TTS streaming funcitonality and example * Update tts addon version * Remove chatterbox example * add new error code for tts streaming fail * Move common code to util * fix: Use z.infer to define TextToSpeechStreamClientParams * Move TextToSpeechStreamSession to schemas * Track subscriber current index and trim queue when all subscribers consumed past items * add missing unit tests * fix: drive done promise from multicast pump lifecycle * fix: Forward chunkIndex and sentenceChunk in sentence-stream mode to client * fix: Use correct error code for tts stream failure * chore: Add supertonic stream test in tts-tests.ts * fix: Make tts client more readable * Remove closures and inline async generators * fix: Subscribe eagerly in sentenceStreamTts to avoid late-subscriber data loss TtsMulticast.pump() starts in a microtask on construction, while the returned async generators only call subscribe() when first iterated. If the consumer iterated one generator before the other, the first subscriber could trim the queue before the second ever registered, silently dropping earlier frames. Subscribe synchronously for both bufferStream and chunkUpdates before returning, so both subscriber indexes are in place before pump pushes its first item. Made-with: Cursor * fix: Close TTS stream on server-sent done frame Remove the dead `null` sentinel from `processTextToSpeechStreamLine` and instead close `parseTextToSpeechStreamLines` after yielding the terminal `done: true` frame, so consumers don't rely on the server closing the socket to stop iteration. Made-with: Cursor * fix: Reject sentenceStream without stream in textToSpeech Previously `sentenceStream: true` combined with `stream: false` fell through to the collect path, silently dropping the sentence-stream parameters and returning no `chunkUpdates`. Fail fast at the dispatcher with a clear error so the contract mismatch surfaces to the caller instead of being swallowed. Made-with: Cursor * fix: Release TtsMulticast subscriber slot on early break Wire a try/finally into drain() so that when a consumer breaks out of the for-await (or the generator is .return()'d / throws), the slot is parked at +Infinity via unsubscribe(). This prevents a stale low min-index from permanently pinning trimConsumed, which otherwise leaked the queue for the entire RPC stream. Made-with: Cursor * fix: Guard TTS stream write after close and preserve UTF-8 boundaries Client: - Track a `closed` flag in `textToSpeechStream` duplex session, set by `end()` / `destroy()`. Subsequent `write()` calls now throw a typed `TextToSpeechStreamFailedError` instead of propagating a raw Bare/Node "write after end" stream error. - `end()` is idempotent so accidental double-close no longer errors. Server: - `buffersToUtf8Fragments` previously decoded each incoming Buffer via `toString("utf8")`, which corrupts any multi-byte codepoint whose bytes straddle a chunk boundary (common with CJK / emoji / accented scripts emitted as LLM token deltas). Added a small tail-buffer that finds the last complete UTF-8 codepoint end in the combined buffer and defers trailing incomplete bytes to the next chunk. Any dangling partial sequence is flushed on stream end. Made-with: Cursor * fix: Order TEXT_TO_SPEECH_STREAM_FAILED code and document it - Move TEXT_TO_SPEECH_STREAM_FAILED (52415) to the end of the 52400 Model Operations block so the ordering in SDK_SERVER_ERROR_CODES matches the numeric sequence (…52413, 52414, 52415). - Add the missing row for 52415 to the (latest) errors.mdx table, per the sdk/docs-freshness rule that the error table stay in sync whenever a new code is introduced. Made-with: Cursor * fix: Register operation metrics for textToSpeechStream Only `textToSpeech` was registered in `operation-metrics.ts`, so the duplex `textToSpeechStream` path silently skipped `modelExecutionTime`, `audioDuration`, and `totalSamples` gauges even though the server already collects the same `TtsStats` via `collectTtsStats()` on the final chunk. Mirror the non-streaming registration so the streaming path has parity observability. Made-with: Cursor * fix: Harden TTS client done-promise, iterator, and parse cost Polish the remaining review nits on the TTS client streaming surface. - #3 TtsMulticast.pump now rejects the `done` promise with the fatal error instead of resolving `false`. An internal `.catch(() => {})` silences unhandled-rejection warnings when the caller only iterates the buffer/chunk streams and never awaits `done`; re-awaits still see the rejection. - #6 TextToSpeechStreamSession[Symbol.asyncIterator] no longer throws synchronously on a second iteration; it returns an iterator whose first `.next()` rejects, so `for await` surfaces the error in the normal async control flow rather than the iterator protocol. - #9 plainTtsBufferStream / collectTtsBuffer wrap the RPC loop in try/catch/finally so `done` always settles: resolve(true) on the terminal frame, reject with the real error on exceptions, and resolve(false) on early consumer break. Previously `await done` could hang forever when the consumer bailed out early. - #11 Skip per-frame ttsResponseSchema.parse() in all three paths; rely on the discriminated-union narrowing at the RPC boundary. Drops the per-PCM-frame Zod validation cost for large sentences. Made-with: Cursor * fix: Tighten textToSpeechStream schema surface - Add .positive() to maxBufferScalars and flushAfterMs to match the existing constraint on sentenceStreamMaxChunkScalars. Previously a caller could pass negative values straight through to the addon. - Un-export textToSpeechStreamRequestBaseSchema — consumers only need the finalized textToSpeechStreamRequestSchema, and the base is an implementation detail of the shared object shape. The exported type alias TextToSpeechStreamClientParams continues to derive from the base via `typeof`, so nothing on the public type surface changes. Made-with: Cursor * fix: Cross-platform tmp path and safer PCM append in TTS examples - playPcmInt16Chunk now writes the intermediate WAV chunk under os.tmpdir() / path.join instead of a hard-coded /tmp/qvac-tts-chunk-… path. The previous code's Windows branch was unreachable in practice because the POSIX /tmp directory doesn't exist there; this uses %TEMP% on Windows automatically. - appendPcmSamples switches from `target.push(...chunk.slice(i, end))` to `Array.prototype.push.apply(target, chunk.slice(i, end))`. Same semantics, but avoids allocating the spread rest array per batch and is closer to a memcpy-style concat in V8. Made-with: Cursor * fix: Catch zero-chunk regressions in TTS sentence-stream test - TtsExecutor.makeSentenceStream now returns `{ passed: false, ... }` when the chunkUpdates iterator yields no chunks / no samples. The previous executor always returned a formatted string regardless of counts, so a regression that silently emitted zero chunks would still have looked like a pass. - ttsSupertonicSentenceStream's expectation upgraded from `{ validation: "type", expectedType: "string" }` to `{ validation: "contains-all", contains: ["sentence-streamed", "chunks", "samples"] }`. The executor's zero-case failure string lacks "sentence-streamed", so the contains-all match fails on regression. Made-with: Cursor * fix: Apply stream default locally and throw typed error on tts mismatch Previous guard only rejected the explicit `stream: false + sentenceStream: true` combination. A caller passing `{ modelId, text, sentenceStream: true }` with `stream` omitted silently fell through to `collectTts` while the server's Zod `.default(true)` still ran the sentence-stream branch and emitted chunk frames — which the client then discarded, dropping all chunk metadata. - Resolve the `stream` default locally (`params.stream ?? true`) so the client's dispatch routing matches the server's Zod-applied routing, and an omitted `stream` now correctly lands in `sentenceStreamTts` or `plainStreamTts`. - Only the explicit `sentenceStream: true + stream: false` combination is rejected, and it now throws `TextToSpeechStreamFailedError` (code 52415) instead of a bare `new Error(...)` so callers can discriminate by error code like everywhere else in the SDK. Made-with: Cursor * remove inline defaults for sentenceStream and stream * Use TtsMulticast in unit test instead of mock --------- Co-authored-by: Ishan Vohra <ishanvohra@Ishans-MacBook-Air.local>
iancris
added a commit
that referenced
this pull request
May 12, 2026
Finding #11: tetherto/temp-8189 fiber fork introduces 19-29% decode regression vs upstream b9025 (Qwen3.5 -28.8%, Gemma4 -19.3%). Finding #10 annotated: addon overhead may reflect fiber regression, not JS binding overhead. Binary archive paths documented. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Proletter
pushed a commit
that referenced
this pull request
May 24, 2026
Finding #11: tetherto/temp-8189 fiber fork introduces 19-29% decode regression vs upstream b9025 (Qwen3.5 -28.8%, Gemma4 -19.3%). Finding #10 annotated: addon overhead may reflect fiber regression, not JS binding overhead. Binary archive paths documented. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Proletter
pushed a commit
that referenced
this pull request
May 24, 2026
…peech (#1590) * feat: Add runStream() which takes input as a stream * add integration tests * uncomment cb tests * chore: Add cb streaming example * feat: Add TTS streaming funcitonality and example * Update tts addon version * Remove chatterbox example * add new error code for tts streaming fail * Move common code to util * fix: Use z.infer to define TextToSpeechStreamClientParams * Move TextToSpeechStreamSession to schemas * Track subscriber current index and trim queue when all subscribers consumed past items * add missing unit tests * fix: drive done promise from multicast pump lifecycle * fix: Forward chunkIndex and sentenceChunk in sentence-stream mode to client * fix: Use correct error code for tts stream failure * chore: Add supertonic stream test in tts-tests.ts * fix: Make tts client more readable * Remove closures and inline async generators * fix: Subscribe eagerly in sentenceStreamTts to avoid late-subscriber data loss TtsMulticast.pump() starts in a microtask on construction, while the returned async generators only call subscribe() when first iterated. If the consumer iterated one generator before the other, the first subscriber could trim the queue before the second ever registered, silently dropping earlier frames. Subscribe synchronously for both bufferStream and chunkUpdates before returning, so both subscriber indexes are in place before pump pushes its first item. Made-with: Cursor * fix: Close TTS stream on server-sent done frame Remove the dead `null` sentinel from `processTextToSpeechStreamLine` and instead close `parseTextToSpeechStreamLines` after yielding the terminal `done: true` frame, so consumers don't rely on the server closing the socket to stop iteration. Made-with: Cursor * fix: Reject sentenceStream without stream in textToSpeech Previously `sentenceStream: true` combined with `stream: false` fell through to the collect path, silently dropping the sentence-stream parameters and returning no `chunkUpdates`. Fail fast at the dispatcher with a clear error so the contract mismatch surfaces to the caller instead of being swallowed. Made-with: Cursor * fix: Release TtsMulticast subscriber slot on early break Wire a try/finally into drain() so that when a consumer breaks out of the for-await (or the generator is .return()'d / throws), the slot is parked at +Infinity via unsubscribe(). This prevents a stale low min-index from permanently pinning trimConsumed, which otherwise leaked the queue for the entire RPC stream. Made-with: Cursor * fix: Guard TTS stream write after close and preserve UTF-8 boundaries Client: - Track a `closed` flag in `textToSpeechStream` duplex session, set by `end()` / `destroy()`. Subsequent `write()` calls now throw a typed `TextToSpeechStreamFailedError` instead of propagating a raw Bare/Node "write after end" stream error. - `end()` is idempotent so accidental double-close no longer errors. Server: - `buffersToUtf8Fragments` previously decoded each incoming Buffer via `toString("utf8")`, which corrupts any multi-byte codepoint whose bytes straddle a chunk boundary (common with CJK / emoji / accented scripts emitted as LLM token deltas). Added a small tail-buffer that finds the last complete UTF-8 codepoint end in the combined buffer and defers trailing incomplete bytes to the next chunk. Any dangling partial sequence is flushed on stream end. Made-with: Cursor * fix: Order TEXT_TO_SPEECH_STREAM_FAILED code and document it - Move TEXT_TO_SPEECH_STREAM_FAILED (52415) to the end of the 52400 Model Operations block so the ordering in SDK_SERVER_ERROR_CODES matches the numeric sequence (…52413, 52414, 52415). - Add the missing row for 52415 to the (latest) errors.mdx table, per the sdk/docs-freshness rule that the error table stay in sync whenever a new code is introduced. Made-with: Cursor * fix: Register operation metrics for textToSpeechStream Only `textToSpeech` was registered in `operation-metrics.ts`, so the duplex `textToSpeechStream` path silently skipped `modelExecutionTime`, `audioDuration`, and `totalSamples` gauges even though the server already collects the same `TtsStats` via `collectTtsStats()` on the final chunk. Mirror the non-streaming registration so the streaming path has parity observability. Made-with: Cursor * fix: Harden TTS client done-promise, iterator, and parse cost Polish the remaining review nits on the TTS client streaming surface. - #3 TtsMulticast.pump now rejects the `done` promise with the fatal error instead of resolving `false`. An internal `.catch(() => {})` silences unhandled-rejection warnings when the caller only iterates the buffer/chunk streams and never awaits `done`; re-awaits still see the rejection. - #6 TextToSpeechStreamSession[Symbol.asyncIterator] no longer throws synchronously on a second iteration; it returns an iterator whose first `.next()` rejects, so `for await` surfaces the error in the normal async control flow rather than the iterator protocol. - #9 plainTtsBufferStream / collectTtsBuffer wrap the RPC loop in try/catch/finally so `done` always settles: resolve(true) on the terminal frame, reject with the real error on exceptions, and resolve(false) on early consumer break. Previously `await done` could hang forever when the consumer bailed out early. - #11 Skip per-frame ttsResponseSchema.parse() in all three paths; rely on the discriminated-union narrowing at the RPC boundary. Drops the per-PCM-frame Zod validation cost for large sentences. Made-with: Cursor * fix: Tighten textToSpeechStream schema surface - Add .positive() to maxBufferScalars and flushAfterMs to match the existing constraint on sentenceStreamMaxChunkScalars. Previously a caller could pass negative values straight through to the addon. - Un-export textToSpeechStreamRequestBaseSchema — consumers only need the finalized textToSpeechStreamRequestSchema, and the base is an implementation detail of the shared object shape. The exported type alias TextToSpeechStreamClientParams continues to derive from the base via `typeof`, so nothing on the public type surface changes. Made-with: Cursor * fix: Cross-platform tmp path and safer PCM append in TTS examples - playPcmInt16Chunk now writes the intermediate WAV chunk under os.tmpdir() / path.join instead of a hard-coded /tmp/qvac-tts-chunk-… path. The previous code's Windows branch was unreachable in practice because the POSIX /tmp directory doesn't exist there; this uses %TEMP% on Windows automatically. - appendPcmSamples switches from `target.push(...chunk.slice(i, end))` to `Array.prototype.push.apply(target, chunk.slice(i, end))`. Same semantics, but avoids allocating the spread rest array per batch and is closer to a memcpy-style concat in V8. Made-with: Cursor * fix: Catch zero-chunk regressions in TTS sentence-stream test - TtsExecutor.makeSentenceStream now returns `{ passed: false, ... }` when the chunkUpdates iterator yields no chunks / no samples. The previous executor always returned a formatted string regardless of counts, so a regression that silently emitted zero chunks would still have looked like a pass. - ttsSupertonicSentenceStream's expectation upgraded from `{ validation: "type", expectedType: "string" }` to `{ validation: "contains-all", contains: ["sentence-streamed", "chunks", "samples"] }`. The executor's zero-case failure string lacks "sentence-streamed", so the contains-all match fails on regression. Made-with: Cursor * fix: Apply stream default locally and throw typed error on tts mismatch Previous guard only rejected the explicit `stream: false + sentenceStream: true` combination. A caller passing `{ modelId, text, sentenceStream: true }` with `stream` omitted silently fell through to `collectTts` while the server's Zod `.default(true)` still ran the sentence-stream branch and emitted chunk frames — which the client then discarded, dropping all chunk metadata. - Resolve the `stream` default locally (`params.stream ?? true`) so the client's dispatch routing matches the server's Zod-applied routing, and an omitted `stream` now correctly lands in `sentenceStreamTts` or `plainStreamTts`. - Only the explicit `sentenceStream: true + stream: false` combination is rejected, and it now throws `TextToSpeechStreamFailedError` (code 52415) instead of a bare `new Error(...)` so callers can discriminate by error code like everywhere else in the SDK. Made-with: Cursor * remove inline defaults for sentenceStream and stream * Use TtsMulticast in unit test instead of mock --------- Co-authored-by: Ishan Vohra <ishanvohra@Ishans-MacBook-Air.local>
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.
No description provided.