additional support for enabling noise isolation#5
Conversation
There was a problem hiding this comment.
Pull request overview
Adds SDK-level support for enabling/disabling server-side noise isolation, exposing it via WebSocket session injection, REST call updates, and pipeline verb typing/schema.
Changes:
- Add
Session.injectNoiseIsolation(...)WebSocket inject helper. - Add REST API support via
UpdateCallRequestfields andCallsResource.noiseIsolation(...)convenience method. - Add
noiseIsolationsupport to thepipelineverb TypeScript types and JSON Schema.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| typescript/src/websocket/session.ts | Adds a WebSocket inject helper for toggling noise isolation mid-call. |
| typescript/src/types/verbs.ts | Extends PipelineVerb with a noiseIsolation option. |
| typescript/src/types/rest.ts | Extends UpdateCallRequest with noise-isolation related fields. |
| typescript/src/client/api.ts | Adds a REST convenience method for noise isolation updates. |
| schema/verbs/pipeline.schema.json | Adds noiseIsolation to the pipeline verb schema (shorthand + detailed config). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.injectCommand('noiseIsolation:status', { | ||
| noise_isolation_status: status, | ||
| ...opts, | ||
| }, callSid); |
There was a problem hiding this comment.
injectNoiseIsolation spreads opts directly, which sends keys like vendor/level/model. Elsewhere (REST UpdateCallRequest and CallsResource.noiseIsolation) the API uses noise_isolation_vendor, noise_isolation_level, and noise_isolation_model. To avoid the server ignoring these fields, map the option keys to the expected noise_isolation_* names (and omit undefined values) before calling injectCommand.
| this.injectCommand('noiseIsolation:status', { | |
| noise_isolation_status: status, | |
| ...opts, | |
| }, callSid); | |
| const payload: Record<string, unknown> = { | |
| noise_isolation_status: status, | |
| }; | |
| if (opts?.vendor !== undefined) { | |
| payload.noise_isolation_vendor = opts.vendor; | |
| } | |
| if (opts?.level !== undefined) { | |
| payload.noise_isolation_level = opts.level; | |
| } | |
| if (opts?.model !== undefined) { | |
| payload.noise_isolation_model = opts.model; | |
| } | |
| this.injectCommand('noiseIsolation:status', payload, callSid); |
| /** Enable or disable server-side noise isolation. */ | ||
| async noiseIsolation( | ||
| callSid: string, | ||
| status: 'enable' | 'disable', | ||
| opts?: { vendor?: string; level?: number; model?: string } | ||
| ): Promise<void> { | ||
| return this.update(callSid, { | ||
| noise_isolation_status: status, | ||
| ...(opts?.vendor ? { noise_isolation_vendor: opts.vendor } : {}), | ||
| ...(opts?.level !== undefined ? { noise_isolation_level: opts.level } : {}), | ||
| ...(opts?.model ? { noise_isolation_model: opts.model } : {}), | ||
| }); | ||
| } |
There was a problem hiding this comment.
There are existing tests for CallsResource convenience methods (redirect/whisper/mute), but the new noiseIsolation convenience method is untested. Add a unit test to verify it sends noise_isolation_status and correctly maps opts.vendor/level/model to noise_isolation_vendor/noise_isolation_level/noise_isolation_model in the request body.
| "direction": { | ||
| "type": "string", | ||
| "enum": ["read", "write"], | ||
| "description": "Audio direction to apply noise isolation. 'read' filters caller audio, 'write' filters outbound audio. Default: 'read'." | ||
| }, |
There was a problem hiding this comment.
The direction option supports both read (caller audio) and write (outbound audio), but the noiseIsolation description frames this as only applying to the caller's audio. Please update the description to reflect both directions (or clarify defaults/limitations) to avoid misleading schema consumers.
No description provided.