Fix: [for cherry-picking] Added WaveClient.postArrayBuffer() with share - #68
Fix: [for cherry-picking] Added WaveClient.postArrayBuffer() with share#68qodo-code-review[bot] wants to merge 1 commit into
Conversation
|
PR author is in the excluded authors list. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 synthesizeStream still uses a bare fetch and private config access
synthesizeStream continues to bypass WaveClient entirely (this.client['config']), so it gets no retries, rate-limit handling, timeout, organization header, custom headers, or WaveError typing — the exact problem this PR fixes for synthesize. It also posts the full request body to /v1/voice/synthesize/stream, which is inconsistent with the whitelisted { text, voice_id } body now used by synthesize against /v1/voice. Worth confirming which contract the live gateway actually implements.
(Refers to lines 203-217)
Was this helpful? React with 👍 or 👎 to provide feedback.
| return this.client.postArrayBuffer(this.basePath, { | ||
| text: request.text, | ||
| ...(request.voice_id ? { voice_id: request.voice_id } : {}), | ||
| }, { | ||
| headers: { Accept: 'audio/mpeg' }, | ||
| responseType: 'arraybuffer', | ||
| }); |
There was a problem hiding this comment.
🔴 Audio options in a speech request are silently ignored
Speech synthesis now sends only the text and the voice choice (postArrayBuffer at src/voice.ts:159-162), dropping every other setting the caller supplied, so requested format, sample rate, speed, pitch, volume and style are silently discarded.
Impact: Users who ask for a specific audio format or speaking style get default audio instead, with no error.
Whitelisted body construction drops documented request fields
SynthesizeRequest (src/voice-types.ts:25-50) accepts format, sample_rate, speed, pitch, volume, ssml, stability, similarity_boost, style, webhook_url. The previous implementation forwarded the whole request object; the new code builds { text, voice_id? } only. CHANGELOG.md still states "the full SynthesizeRequest (including audio options) is forwarded", so behavior and docs now disagree.
| return this.client.postArrayBuffer(this.basePath, { | |
| text: request.text, | |
| ...(request.voice_id ? { voice_id: request.voice_id } : {}), | |
| }, { | |
| headers: { Accept: 'audio/mpeg' }, | |
| responseType: 'arraybuffer', | |
| }); | |
| return this.client.postArrayBuffer(this.basePath, request, { | |
| headers: { Accept: 'audio/mpeg' }, | |
| }); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| options: RequestOptions = {}, | ||
| responseType: 'json' | 'arrayBuffer' = 'json' | ||
| ): Promise<T> { | ||
| const { params, noRetry, timeout: requestTimeout, responseType, ...fetchOptions } = options; | ||
| const { params, noRetry, timeout: requestTimeout, ...fetchOptions } = options; |
There was a problem hiding this comment.
🟡 Public option for requesting raw binary responses no longer has any effect
The publicly documented request option that asks for raw bytes is no longer read by the request path (destructuring at src/client.ts:274 dropped it), so any caller using it now receives an empty object instead of the binary data.
Impact: Existing SDK users who request binary responses get empty results instead of audio/file bytes.
Option remains in the public type but is unused
RequestOptions.responseType?: 'json' | 'arraybuffer' is still declared and documented in src/client-types.ts:29-32, but request() no longer destructures it; instead the binary mode is selected only via the new third positional parameter (src/client.ts:271-272). A caller-supplied responseType now falls into ...fetchOptions and is passed to fetch as an ignored property, and the response falls through to the non-JSON branch returning {} (src/client.ts:370). Either remove the option from RequestOptions or honor it (note the casing mismatch: the type uses 'arraybuffer' while the new parameter uses 'arrayBuffer').
Prompt for agents
src/client-types.ts still exports RequestOptions.responseType ('json' | 'arraybuffer') as a documented way to request binary responses, but src/client.ts request() no longer reads it after the refactor to a positional responseType parameter ('json' | 'arrayBuffer'). Any external caller passing responseType: 'arraybuffer' now silently gets {} back. Decide whether to keep honoring the option (falling back to it when the positional argument is not given, and reconciling the 'arraybuffer' vs 'arrayBuffer' spelling) or to remove it from the public RequestOptions type and document the change.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return this.client.postArrayBuffer(this.basePath, { | ||
| text: request.text, | ||
| ...(request.voice_id ? { voice_id: request.voice_id } : {}), | ||
| }, { |
There was a problem hiding this comment.
🟡 Changelog not updated for a user-facing behavior change
The user-facing change that stops forwarding audio options for speech synthesis is not reflected in the changelog, which still claims the opposite, violating the repository rule that user-facing changes update the Unreleased section.
Impact: Users reading release notes will believe audio options are still supported.
Rule and stale entry
AGENTS.md requires: "Conventional Commit titles; update CHANGELOG.md (Unreleased) for user-facing changes." The existing Unreleased entry states "the full SynthesizeRequest (including audio options) is forwarded", which is no longer true after src/voice.ts:159-162.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Parse response | ||
| if (responseType === 'arrayBuffer') { | ||
| return response.arrayBuffer() as Promise<T>; | ||
| } |
There was a problem hiding this comment.
🔍 Binary body read is no longer awaited inside the retry try-block
The binary branch now returns the unresolved promise (return response.arrayBuffer() as Promise<T>) instead of awaiting it as the previous code did (return (await response.arrayBuffer()) as T). If the body stream errors mid-read (truncated/aborted connection), the rejection escapes the try in executeWithRetry and can no longer be classified/retried by the catch block at src/client.ts:352-374; the caller sees a raw network error rather than a retry or a WaveError. This mirrors the existing JSON branch behavior, so it is consistent, but it is a behavior regression relative to the prior arraybuffer handling.
Was this helpful? React with 👍 or 👎 to provide feedback.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review Unresolved review comments identify substantive bugs: audio synthesis options are silently dropped, and the public responseType option no longer functions. Additionally, the author does not own these files (owned by wave-av/core-team). Human review required to address these behavioral regressions. You can customize Macroscope's approvability policy. Learn more. |
Fixed Findings
Automated fix from agentic review of #67
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Add
WaveClient.postArrayBuffer()for binary POST responses in voice synthesispostArrayBuffer()toWaveClientin src/client.ts, which POSTs and parses the response as anArrayBufferinstead of JSON. TheresponseTypeis now passed as an explicit third argument torequest()rather than read fromRequestOptions.VoiceAPI.synthesizein src/voice.ts to usepostArrayBuffer(), and narrows the request body to only{ text, voice_id }.executeWithRetryto recognize'arrayBuffer'(camelCase) and returnresponse.arrayBuffer()correctly.VoiceAPI.synthesizeno longer forwards extra fields fromSynthesizeRequestbeyondtextandvoice_idto the API.Macroscope summarized 47661c7.