feat(core): [WIP] proof of concept of core and sdk separation inside the same repo #431
Conversation
- Move existing src/ code to src/core/ (transport layer) - Create new src/sdk/ layer with enhanced functionality - Add v2 listen supervision with turn counting and reconnection - Add middleware system with before/after hooks - Implement version validation at core level - Fix all test import paths and ESLint configuration - Maintain 100% backward compatibility - Add examples for v2 middleware and version validation
- Add WebSocket mocking in unit tests to prevent real connections - Add proper connection cleanup with .finish() calls - Add afterEach cleanup hooks to clear mocks - Fix test suite from 53/54 to 54/54 passing (100% success rate) - Resolve WebSocket connection leaks that were interfering with other tests - Unit tests now properly isolated and don't depend on external services
WalkthroughAdds a core layer with per-API version validation, introduces an SDK layer that enhances Listen with supervision and middleware, restructures public exports to use the sdk entry, updates tests to new core paths, adds Supervisor and MiddlewareRegistry, multiple examples/demos, and adjusts ESLint with test overrides. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant SDK as SDK DeepgramClient
participant Core as Core DeepgramClient
participant Listen as ListenClient
participant Live as ListenLiveClient
participant Sup as Supervisor
App->>SDK: createClient(...)/v("v2")
SDK->>Core: construct Core client
App->>SDK: client.listen.live(options)
SDK->>Listen: get listen (override)
Listen->>Live: new CoreListenLiveClient(options)
alt version == v2
Listen->>Sup: attach(session, {clientType:"listen",version:"v2"})
Sup-->>Live: inject use(), wrap emit(), register middlewares
end
App-->>Live: on(Open/Transcript/...)
Live-->>App: emit events (middleware before/after executed)
sequenceDiagram
participant App
participant Core as Core DeepgramClient
App->>Core: client.v("v2").manage
Core->>Core: validateVersion("manage","v2")
Core-->>App: throw DeepgramError("not supported")
App->>Core: client.v("v2").listen
Core->>Core: validateVersion("listen","v2")
Core-->>App: return Listen client
sequenceDiagram
participant Live as Live Session
participant Sup as Supervisor
participant Reg as MiddlewareRegistry
participant MW as Middleware
Live-->>Sup: emit(event, payload)
Sup->>Reg: getMiddlewares(session,event)
Reg-->>Sup: [mw1, mw2, ...]
loop before hooks
Sup->>MW: mw.before(payload, ctx)
end
Sup-->>Live: proceed emit to listeners
loop after hooks
Sup->>MW: mw.after(payload, ctx)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
tests/e2e/models-get-model.test.ts (1)
2-3: Same verification and optional barrel suggestion as aboveConfirm the exports resolve under ts-jest and consider a core barrel to avoid deep internal paths. The verification script provided in models-get-all applies here as well.
🧹 Nitpick comments (59)
tests/e2e/models-get-all.test.ts (1)
2-3: ✅ ModelsRestClient & DeepgramError exports verified
ModelsRestClient is exported in src/core/packages/ModelsRestClient.ts and DeepgramError in src/core/lib/errors.ts.
Both files exist and no stale deep-import paths were found—tests will resolve these imports under ts-jest as-is.
Optional: for cleaner, less brittle imports you can add a barrel at src/core/index.ts:
// src/core/index.ts export { ModelsRestClient } from "./packages/ModelsRestClient"; export { DeepgramError } from "./lib/errors";Then simplify your test imports:
-import { ModelsRestClient } from "../../src/core/packages/ModelsRestClient"; -import { DeepgramError } from "../../src/core/lib/errors"; +import { ModelsRestClient, DeepgramError } from "../../src/core";[optional_refactors_recommended]
tests/e2e/speak-live-connection.test.ts (1)
1-3: Reduce brittleness: import via a stable barrel or TS path aliasDeep relative paths couple tests to internal layout. Prefer a stable core entrypoint (barrel) or a TS path alias. If you have (or plan) src/core/index.ts that re-exports public types, consolidate to a single import.
Apply if a core barrel exists:
-import { SpeakLiveClient } from "../../src/core/packages/SpeakLiveClient"; -import { LiveTTSEvents } from "../../src/core/lib/enums/LiveTTSEvents"; -import { CONNECTION_STATE } from "../../src/core/lib/constants"; +import { SpeakLiveClient, LiveTTSEvents, CONNECTION_STATE } from "../../src/core";Alternatively, add a TS path alias to decouple tests from relative paths (tsconfig.json):
{ "compilerOptions": { "baseUrl": ".", "paths": { "@core/*": ["src/core/*"] } } }Then import via the alias (still works without a barrel, but alias keeps paths stable):
-import { SpeakLiveClient } from "../../src/core/packages/SpeakLiveClient"; -import { LiveTTSEvents } from "../../src/core/lib/enums/LiveTTSEvents"; -import { CONNECTION_STATE } from "../../src/core/lib/constants"; +import { SpeakLiveClient } from "@core/packages/SpeakLiveClient"; +import { LiveTTSEvents } from "@core/lib/enums/LiveTTSEvents"; +import { CONNECTION_STATE } from "@core/lib/constants";If the long-term goal is to test the public SDK surface, consider importing from the sdk entrypoint instead, once available, to ensure E2E tests validate the intended public API boundary.
tests/e2e/manage-update-project-member-scope.test.ts (1)
4-4: Prefer path alias or public re-exports to avoid brittle deep relative pathsDirectly importing from
../../src/core/lib/typesmakes tests tightly coupled to internal layout. Consider:
- Re-exporting these types from the SDK entrypoint (or a test-facing barrel), or
- Adding a path alias in tsconfig (e.g.,
@core/* -> src/core/*) to stabilize imports.Example tsconfig addition (for repo root tsconfig or tests tsconfig):
{ "compilerOptions": { "baseUrl": ".", "paths": { "@core/*": ["src/core/*"] } } }Then update the import in this file:
-import type { DeepgramResponse, MessageResponse } from "../../src/core/lib/types"; +import type { DeepgramResponse, MessageResponse } from "@core/lib/types";Alternatively, if you expose types from the SDK root, a stable import like
from "../../src"orfrom "@sdk"via re-exports keeps tests aligned with the public API surface:// e.g., in src/index.ts (SDK entrypoint) export type { DeepgramResponse, MessageResponse } from "./core/lib/types";tests/__utils__/websocket-mocks.ts (1)
2-2: Importing SOCKET_STATES from core constants is fineThis keeps test mocks aligned with the runtime’s symbolic states.
Minor: to decouple tests from internal paths, you could inline the WebSocket readyState numeric values (0..3 per the standard) or define a local enum. This prevents test utilities from breaking if the internal constants path changes again. Keeping the import is also reasonable if you prefer strict parity with the implementation.
tests/unit/url-builders.test.ts (1)
1-1: buildRequestUrl export verified
- Confirmed that
buildRequestUrlis publicly exported fromsrc/core/lib/helpers.ts:99.- Existing tests continue to cover the core encoding scenarios.
Consider adding optional test cases for:
• Reserved characters (spaces,&,/,%)
• Deterministic ordering of query parameters[optional_refactors_recommended]
tests/e2e/manage-get-project-usage-requests.test.ts (1)
4-4: Decouple tests from core internals by sourcing types from the public SDK.Same rationale as other E2E specs: import types via the SDK entrypoint to maintain layering boundaries and reduce future refactors.
Once types are re-exported from the SDK entrypoint, update this line:
-import type { DeepgramResponse, GetProjectUsageRequestsResponse } from "../../src/core/lib/types"; +import type { DeepgramResponse, GetProjectUsageRequestsResponse } from "../../src/index";tests/e2e/manage-get-project-invites.test.ts (1)
4-4: Prefer SDK entrypoint for types to avoid test coupling to core.Re-export the types from src/index.ts and update this import to reduce exposure to internal paths.
Proposed change after re-exports:
-import type { DeepgramResponse, GetProjectInvitesResponse } from "../../src/core/lib/types"; +import type { DeepgramResponse, GetProjectInvitesResponse } from "../../src/index";tests/e2e/__mocks__/manage.ts (1)
25-25: Update deep import to use core barrelSince
src/core/index.tsalready re-exports./lib/types, switch the deep-path import in your mock to the barrel to reduce coupling:• tests/e2e/mocks/manage.ts (line 25)
- } from "../../../src/core/lib/types"; + } from "../../../src/core";[optional_refactors_recommended]
tests/unit/options-transformation.test.ts (1)
1-1: Optionally re-export convertLegacyOptions from the core barrelIt looks like
convertLegacyOptionsisn’t currently exposed viasrc/core/index.ts. If you’d prefer to import it from the public core entry point (to reduce deep-path churn), you can:• Add this export to
src/core/index.ts:export { convertLegacyOptions } from "./lib/helpers";• Then update your test import in
tests/unit/options-transformation.test.ts:- import { convertLegacyOptions } from "../../src/core/lib/helpers"; + import { convertLegacyOptions } from "../../src/core";This change is entirely optional—no action is required if you’re happy with the existing helper path.
[optional_refactors_recommended]tests/e2e/__mocks__/listen.ts (1)
6-6: EnsureAsyncPrerecordedResponseIs Re-exported from the Core BarrelThe type itself lives in
src/core/lib/types/AsyncPrerecordedResponse.tsbut isn’t currently re-exported from your barrel (e.g.src/core/index.ts). Before changing the import in your tests, you’ll need to expose it at the barrel level:– Verify there’s no existing re-export in
src/core/index.ts.
– If absent, add:// src/core/index.ts +export type { AsyncPrerecordedResponse } from "./lib/types/AsyncPrerecordedResponse";Then you can simplify your test import:
-import type { AsyncPrerecordedResponse } from "../../../src/core/lib/types"; +import type { AsyncPrerecordedResponse } from "../../../src/core";tests/e2e/manage-get-project-usage-summary.test.ts (1)
4-4: Core barrel doesn’t currently export these types—direct import is correctThe tests must import from
src/core/lib/typesbecause neitherDeepgramResponsenorGetProjectUsageSummaryResponseare re-exported by the core barrel. If you’d prefer a single-path import, add these exports tosrc/core/index.ts:• In
src/core/index.ts:export type { DeepgramResponse } from "./lib/types/DeepgramResponse"; export type { GetProjectUsageSummaryResponse } from "./lib/types/GetProjectUsageSummaryResponse";Then tests can use:
-import type { DeepgramResponse, GetProjectUsageSummaryResponse } from "../../src/core/lib/types"; +import type { DeepgramResponse, GetProjectUsageSummaryResponse } from "../../src/core";[optional_refactors_recommended]
tests/e2e/selfhosted-list-credentials.test.ts (1)
2-3: Imports Are Correct—Add Barrel Exports If You’d Like a Shorter PathSince neither
SelfHostedRestClientnorDeepgramErrorare currently re-exported fromsrc/core/index.ts, you must keep the deep imports for now. If you’d like to simplify the import paths in the future, consider adding these lines to the core barrel:// src/core/index.ts export { SelfHostedRestClient } from "./packages/SelfHostedRestClient"; export { DeepgramError } from "./lib/errors";Then you can switch your test imports to:
import { SelfHostedRestClient, DeepgramError } from "../../src/core";[optional_refactors_recommended]
tests/e2e/manage-get-project-member-scopes.test.ts (1)
4-4: Core barrel missing these type exports—add them before switching importsTo import
DeepgramResponseandGetProjectMemberScopesResponsefrom the core barrel, first re-export them insrc/core/index.ts. For example:--- a/src/core/index.ts +++ b/src/core/index.ts @@ + export type { DeepgramResponse, GetProjectMemberScopesResponse } from './lib/types';Once that’s in place, update the test import in
tests/e2e/manage-get-project-member-scopes.test.ts:-import type { DeepgramResponse, GetProjectMemberScopesResponse } from "../../src/core/lib/types"; +import type { DeepgramResponse, GetProjectMemberScopesResponse } from "../../src/core";[optional_refactors_recommended]
examples/node-listen-v2-middleware-test/package.json (1)
1-13: Mark example package as private and set minimum Node version.To avoid accidental publishes and clarify runtime expectations, mark this example as private and declare a Node engine (Node 18+ recommended due to fetch and WebSocket ecosystem changes).
Apply:
{ "name": "node-listen-v2-middleware-test", "version": "1.0.0", "description": "Test v2 middleware functionality on v1 endpoint", "main": "index.js", + "private": true, + "license": "UNLICENSED", + "engines": { + "node": ">=18" + }, "scripts": { "start": "node index.js" }, "dependencies": { "cross-fetch": "^3.1.5", "dotenv": "^16.0.0" } }examples/node-listen-v2-enhanced/package.json (1)
1-12: Lock down example package metadata (private, license, engines).Same rationale as the other example: prevent publish and document required Node version.
Apply:
{ "name": "node-listen-v2-enhanced", "version": "1.0.0", "description": "Example of enhanced Listen v2 with middleware and supervision", "main": "index.js", + "private": true, + "license": "UNLICENSED", + "engines": { + "node": ">=18" + }, "scripts": { "start": "node index.js" }, "dependencies": { "dotenv": "^16.0.0" } }tests/e2e/listen-transcribe-url-callback.test.ts (1)
4-5: Types and helpers moved to core path look good; consider re-exporting to reduce deep importsIf the long-term public surface intends fewer deep imports, consider re-exporting
CallbackUrland common types from a core or sdk entrypoint to simplify test imports. Not blocking for this PoC.tests/e2e/manage-create-project-key.test.ts (1)
4-4: Prefer stable core barrel export over deep path to reduce test brittlenessImport these types from the public core barrel instead of the internal
lib/typespath:In tests/e2e/manage-create-project-key.test.ts:
-import type { DeepgramResponse, CreateProjectKeyResponse } from "../../src/core/lib/types"; +import type { DeepgramResponse, CreateProjectKeyResponse } from "../../src/core";[optional_refactors_recommended]
tests/unit/live-client-connection-state.test.ts (1)
1-5: Add missing exports to the core barrel and consolidate deep importsWe verified that all six symbols exist under
src/corebut none are currently re-exported fromsrc/core/index.ts. To minimize coupling and improve readability, please:• In
src/core/index.ts, add:export { ListenLiveClient } from "./packages/ListenLiveClient"; export { SpeakLiveClient } from "./packages/SpeakLiveClient"; export { AgentLiveClient } from "./packages/AgentLiveClient"; export { CONNECTION_STATE, SOCKET_STATES } from "./lib/constants"; export { LiveTranscriptionEvents } from "./lib/enums/LiveTranscriptionEvents";• In
tests/unit/live-client-connection-state.test.ts, replace the five separate imports with:-import { ListenLiveClient } from "../../src/core/packages/ListenLiveClient"; -import { SpeakLiveClient } from "../../src/core/packages/SpeakLiveClient"; -import { AgentLiveClient } from "../../src/core/packages/AgentLiveClient"; -import { CONNECTION_STATE, SOCKET_STATES } from "../../src/core/lib/constants"; -import { LiveTranscriptionEvents } from "../../src/core/lib/enums/LiveTranscriptionEvents"; +import { + ListenLiveClient, + SpeakLiveClient, + AgentLiveClient, + CONNECTION_STATE, + SOCKET_STATES, + LiveTranscriptionEvents, +} from "../../src/core";This will give you a stable, single-point import surface for all core exports.
[optional_refactors_recommended]tests/unit/fetch-utilities.test.ts (1)
1-2: Core barrel missing exportsPath migration looks correct. However,
src/core/index.tsdoesn’t currently re-export these utilities, so importing via the core barrel will fail. To enable barrel imports and reduce churn in future refactors, consider:
- Adding the following to src/core/index.ts:
+ export { resolveHeadersConstructor } from "./lib/helpers"; + export { resolveFetch, fetchWithAuth, resolveResponse } from "./lib/fetch";- Then updating your test import in tests/unit/fetch-utilities.test.ts:
- import { resolveHeadersConstructor } from "../../src/core/lib/helpers"; - import { resolveFetch, fetchWithAuth, resolveResponse } from "../../src/core/lib/fetch"; + import { resolveHeadersConstructor, resolveFetch, fetchWithAuth, resolveResponse } from "../../src/core";This change is optional but recommended to keep imports concise. [optional_refactors_recommended]
tests/e2e/manage-get-project-usage-request.test.ts (1)
4-4: Add SDK re-exports for core types and update test import
Verified that DeepgramResponse and GetProjectUsageRequestResponse are currently only defined in core and not re-exported bysrc/index.tsorsrc/sdk/index.ts. To maintain SDK/core separation, please:
- In
src/sdk/index.ts, add:export type { DeepgramResponse, GetProjectUsageRequestResponse } from "../core/lib/types";- In
tests/e2e/manage-get-project-usage-request.test.ts, update the import to:import type { DeepgramResponse, GetProjectUsageRequestResponse } from "../../src/sdk";[optional_refactors_recommended]
tests/e2e/listen-transcribe-file-callback.test.ts (1)
4-5: Expose CallbackUrl via SDK for cleaner importsWe verified that
CallbackUrlis defined and exported insrc/core/lib/helpers.ts:89but is not re-exported in eithersrc/index.tsorsrc/sdk/index.ts. To reduce test coupling on core internals and improve SDK ergonomics, consider:
- Re-exporting
CallbackUrlfrom your SDK entry point.
For example, insrc/index.ts(orsrc/sdk/index.ts):export { CallbackUrl } from './core/lib/helpers';- Or update the public SDK method signatures to accept a plain
stringURL and internally wrap it withnew CallbackUrl(...).[optional_refactors_recommended]
tests/unit/type-guards.test.ts (1)
112-112: Runtime import path change is fine; optional: prefer module mocking over require.The
requireusage is pragmatic for spying in Jest. Optionally, you could replace with a module mock (jest.mock) and import at top-level to avoid dynamic require and the ESLint suppression. Not blocking.tests/e2e/manage-get-project-members.test.ts (1)
4-4: Re-export core types in your public API for cleaner E2E importsE2E tests currently import these types directly from
src/core/lib/types, which ties them to internal file paths. To keep your SDK surface stable and decoupled, re-exportDeepgramResponseandGetProjectMembersResponsefrom your root or SDK entrypoint.• In
src/index.ts(orsrc/sdk/index.ts), add:export * from './sdk'; +export type { DeepgramResponse } from './core/lib/types/DeepgramResponse'; +export type { GetProjectMembersResponse } from './core/lib/types/GetProjectMembersResponse';After this, update your test import to:
import type { DeepgramResponse, GetProjectMembersResponse } from "your-sdk";[optional_refactors_recommended]
src/core/packages/AbstractLiveClient.ts (1)
529-534: Tighten types on dummy WebSocket handlers for clarityWhile this is a formatting-only change, the
Functiontype is overly loose and makes handler contracts unclear. Consider minimal, explicit signatures to improve maintainability and editor assistance.Apply within this range:
- onclose: Function = () => {}; - onerror: Function = () => {}; - onmessage: Function = () => {}; - onopen: Function = () => {}; + onclose: (ev?: any) => void = () => {}; + onerror: (ev?: any) => void = () => {}; + onmessage: (ev?: { data?: any } | any) => void = () => {}; + onopen: () => void = () => {}; - send: Function = () => {}; + send: (data?: any) => void = () => {};If you want to propagate this cleanup beyond the selected lines, also consider updating
closeabove to:close: (code?: number, reason?: string) => void;.eslintrc.js (2)
7-9: Enable browser globals or define WebSocket to avoid no-undef in TS filesGiven the code references
WebSocket(e.g., AbstractLiveClient), consider addingbrowser: truetoenv(or explicitly declaringWebSocketas a global) so ESLint doesn’t flag browser globals in TS despite typing. This is especially helpful ifno-undefis active.env: { - node: true, + node: true, + browser: true, },
29-43: ✅ Safe to spread only valid keys (rules,env) from .eslintrc.test.js into your overrideI ran:
node -e 'const c=require("./.eslintrc.test.js"); console.log(Object.keys(c))'and confirmed that
.eslintrc.test.jsexports onlyrulesandenv, both of which are valid within anoverrideblock. No top-level-only keys (likeoverridesorroot) will be pulled in, so your current approach is safe.If you’d like to embrace the more idiomatic, future-proof style, you can optionally switch to using
extendsin the override:overrides: [ { // Apply test-specific rules to test files files: [ "tests/**/*.ts", "tests/**/*.js", "**/*.test.ts", "**/*.test.js", "**/*.spec.ts", "**/*.spec.js", ], - // eslint-disable-next-line @typescript-eslint/no-require-imports - ...require("./.eslintrc.test.js"), + extends: ["./.eslintrc.test.js"], }, ],[optional_refactors_recommended]
examples/node-listen-v2-middleware-test/README.md (3)
60-65: Tighten phrasing in the “Expected Output” listMinor style/grammar polish for consistency and readability.
-1. **Global before middleware** - Logs transcript and adds timestamp -2. **Instance before middleware** - Adds instance marker -3. **User event handler** - Your normal transcript processing -4. **Instance after middleware** - Logs instance marker -5. **Global after middleware** - Logs processing time +1. **Global before middleware** — logs transcript and adds a timestamp +2. **Instance before middleware** — adds an instance marker +3. **User event handler** — runs your normal transcript processing +4. **Instance after middleware** — logs the instance marker +5. **Global after middleware** — logs processing time
68-75: Safer env loading for secretsThe current pipeline writes exports to a temp file and can break on spaces/quotes in values. Prefer dotenv or a POSIX-safe approach that avoids writing secrets to disk.
-# Load environment variables -cat ../../.env | grep -v '^#' | xargs -I {} echo 'export {}' > /tmp/env_vars.sh -source /tmp/env_vars.sh - -# Run the test -npm start +# Run with env from ../../.env using dotenv-cli +npx dotenv -e ../../.env -- npm startIf you want a pure shell option:
set -a # shellcheck disable=SC1091 . ../../.env set +a npm start
81-84: Clarify architecture phrasingMinor language tweak to make layering clearer.
-- **Transport Layer** (v1/v2 API endpoints) - in core -- **Enhancement Layer** (middleware, supervision) - in SDK +- **Transport layer** (v1/v2 API endpoints) — in the core package +- **Enhancement layer** (middleware, supervision) — in the SDK.eslintrc.test.js (1)
20-23: Optional: add Jest plugin rules for richer test lintingIf you want stronger test linting (beyond globals), consider adding plugin:jest/recommended in the test overrides at the top-level .eslintrc.js. This adds useful checks without being too noisy.
examples/test-version-validation/package.json (1)
1-12: Mark example package as private
The example uses CommonJS (onlyrequire()), so do not add"type": "module". To prevent accidental publishing, add"private": trueto thepackage.json:{ "name": "test-version-validation", "version": "1.0.0", "description": "Test version validation in Deepgram SDK", "main": "index.js", + "private": true, "scripts": { "start": "node index.js" }, "dependencies": { "dotenv": "^16.0.0" } }[optional_refactors_recommended]
src/sdk/middleware/index.ts (1)
1-3: Barrel re-exports are fine; consider explicit exports for API stabilityexport * can unintentionally expose internals when new symbols are added. Consider switching to explicit named exports once the surface settles to avoid accidental API creep.
src/sdk/DeepgramClient.ts (1)
16-19: Triggering validation via super.listen has side effects; consider exposing a protected validatorAccessing the parent getter to fire validation constructs and discards a core ListenClient. It’s minor overhead but slightly opaque. Prefer a protected validateVersion("listen") in the core client to avoid instantiating an unused object.
I can open a follow-up to expose a protected validator in core so this reads: this.validateVersion("listen", this.version).
examples/node-listen-v2-middleware-test/index.js (3)
88-109: Make fetch body streaming Node/Web-stream compatible (prevents runtime errors in Node 18+).In Node 18+, fetch returns a WHATWG ReadableStream; calling
.on("readable")onr.bodywill throw. Support both: web streams viagetReader()and Node streams via.on("data"). Also consolidate error/fallback handling.Apply this diff:
- fetch("http://stream.live.vc.bbcmedia.co.uk/bbc_world_service") - .then((r) => r.body) - .then((res) => { - console.log(" 📡 Audio stream connected"); - res.on("readable", () => { - const chunk = res.read(); - if (chunk) { - connection.send(chunk); - } - }); - }) - .catch((err) => { - console.log(" ⚠️ Audio stream failed:", err.message); - // Send some test data instead - setTimeout(() => { - console.log(" 📡 Sending test audio data..."); - const testAudio = Buffer.from("test audio data"); - connection.send(testAudio); - }, 1000); - }); + (async () => { + try { + const response = await fetch("http://stream.live.vc.bbcmedia.co.uk/bbc_world_service"); + const body = response.body; + console.log(" 📡 Audio stream connected"); + if (!body) throw new Error("No response body"); + + // Web Streams API (Node 18+) + if (typeof body.getReader === "function") { + const reader = body.getReader(); + // eslint-disable-next-line no-constant-condition + while (true) { + const { done, value } = await reader.read(); + if (done) break; + if (value) connection.send(Buffer.from(value)); + } + } + // Node.js stream (node-fetch / older runtimes) + else if (typeof body.on === "function") { + body.on("data", (chunk) => connection.send(chunk)); + } else { + console.log(" ⚠️ Unsupported stream type from fetch()"); + } + } catch (err) { + console.log(" ⚠️ Audio stream failed:", err.message); + // Send some test data instead + setTimeout(() => { + console.log(" 📡 Sending test audio data..."); + const testAudio = Buffer.from("test audio data"); + connection.send(testAudio); + }, 1000); + } + })();
24-31: Avoid mutating event payload; prefer passing ephemeral data via middleware context.Writing
_middleware_timestampand_instance_markeron the payload can leak into user-visible objects and create naming collisions. Store ephemeral values inctxinstead and read from there in handlers.Example (within the same ranges):
- before: (payload, ctx) => { + before: (payload, ctx) => { console.log("🔧 BEFORE middleware - Results received:"); // ... - payload._middleware_timestamp = Date.now(); + ctx._middleware_timestamp = Date.now(); }, - after: (payload, ctx) => { - const processingTime = Date.now() - payload._middleware_timestamp; + after: (payload, ctx) => { + const processingTime = Date.now() - (ctx._middleware_timestamp ?? Date.now()); // ... },For instance middleware:
- before: (payload) => { + before: (payload, ctx) => { // ... - payload._instance_marker = "processed-by-instance"; + ctx._instance_marker = "processed-by-instance"; }, - after: (payload) => { + after: (payload, ctx) => { // ... - console.log(" ✅ Instance marker:", payload._instance_marker); + console.log(" ✅ Instance marker:", ctx._instance_marker); },And in the Transcript handler:
- console.log(" 🏷️ Instance marker:", data._instance_marker); - console.log(" ⏱️ Middleware timestamp:", data._middleware_timestamp); + // If you need these here, mirror ctx in the enhanced payload upstream or log within middlewareAlso applies to: 68-81, 119-123
142-149: Prefer exiting after Close event to avoid premature process termination.Calling process.exit(0) on a timer can terminate before the Close callback completes network cleanup. Exit on Close, with a secondary timeout as a safeguard.
Example:
- connection.requestClose(); - setTimeout(() => { - process.exit(0); - }, 1000); + connection.requestClose(); + const forceExit = setTimeout(() => process.exit(0), 5000); + connection.on(LiveTranscriptionEvents.Close, () => { + clearTimeout(forceExit); + process.exit(0); + });examples/test-version-validation/index.js (1)
33-44: Optional: assert v2 listen access rather than just typeof.To fail loudly if access isn’t enabled, touch a nested property or method on the listen client. This avoids a false positive if a proxy/stub returns an object.
Example:
- const listenClient = v2Client.listen; - console.log(" ✅ v2 listen client:", typeof listenClient === "object" ? "works" : "failed"); + const listenClient = v2Client.listen; + // Touch a nested member to ensure the getter fully resolves + console.log( + " ✅ v2 listen client:", + typeof listenClient.live === "function" ? "works" : "failed" + );examples/node-listen-v2-enhanced/index.js (2)
12-21: Consistent enhanced timestamp naming across examples.Elsewhere (demo-enhanced-sdk.js) the property appears as
_enhanced_timestamp. Consider aligning to reduce confusion for users scanning examples.Apply this diff:
- payload._enhanced_at = Date.now(); + payload._enhanced_timestamp = Date.now(); ... - const processingTime = Date.now() - payload._enhanced_at; + const processingTime = Date.now() - payload._enhanced_timestamp;And in the Transcript handler:
- console.log("⏱️ Enhanced timestamp:", data._enhanced_at); + console.log("⏱️ Enhanced timestamp:", data._enhanced_timestamp);Also applies to: 56-60
41-47: Prefer context for ephemeral data instead of mutating payload.Similar to the other example, store transient fields in the middleware context to avoid polluting event payloads.
Example:
- connection.use({ - event: "SpeechStarted", - before: () => { - console.log("🎤 Instance middleware: User started speaking"); - }, - }); + connection.use({ + event: "SpeechStarted", + before: (_payload, _ctx) => { + console.log("🎤 Instance middleware: User started speaking"); + }, + });src/core/DeepgramClient.ts (2)
24-33: Type-safety and future-proofing for supported version map.Mark version arrays as readonly to prevent accidental mutation and gain stricter type checks.
Apply this diff:
- private static readonly SUPPORTED_VERSIONS = { - listen: ["v1", "v2"], // Listen supports both v1 and v2 - speak: ["v1"], // Speak only supports v1 currently - read: ["v1"], // Read only supports v1 currently - manage: ["v1"], // Management API only supports v1 - models: ["v1"], // Models API only supports v1 - auth: ["v1"], // Auth API only supports v1 - selfhosted: ["v1"], // Self-hosted only supports v1 - agent: ["v1"], // Agent API only supports v1 currently - }; + private static readonly SUPPORTED_VERSIONS = { + listen: ["v1", "v2"] as const, // Listen supports both v1 and v2 + speak: ["v1"] as const, // Speak only supports v1 currently + read: ["v1"] as const, // Read only supports v1 currently + manage: ["v1"] as const, // Management API only supports v1 + models: ["v1"] as const, // Models API only supports v1 + auth: ["v1"] as const, // Auth API only supports v1 + selfhosted: ["v1"] as const, // Self-hosted only supports v1 + agent: ["v1"] as const, // Agent API only supports v1 currently + } as const;
38-63: Avoid recomputing the union of supported versions on every access.
validateVersionbuildsallSupportedVersionson every call. Cache it in a static Set or compute once. Also, optionally guard unknown API keys.Apply this diff:
export default class DeepgramClient extends AbstractClient { + private static readonly ALL_SUPPORTED_VERSIONS = new Set( + // Flatten the readonly tuples into a string array + (Object.values(DeepgramClient.SUPPORTED_VERSIONS) as ReadonlyArray<readonly string[]>) + .flat() + ); /** * Validates if a version is supported for a specific API */ private validateVersion(api: string, version: string): void { - // First check if the version exists at all - const allSupportedVersions = new Set<string>(); - Object.values(DeepgramClient.SUPPORTED_VERSIONS).forEach((versions) => { - versions.forEach((v) => allSupportedVersions.add(v)); - }); - - if (!allSupportedVersions.has(version)) { + // First check if the version exists at all + if (!DeepgramClient.ALL_SUPPORTED_VERSIONS.has(version)) { throw new DeepgramError( - `Version '${version}' is not supported by any API. Available versions: ${Array.from( - allSupportedVersions - ).join(", ")}` + `Version '${version}' is not supported by any API. Available versions: ${Array.from(DeepgramClient.ALL_SUPPORTED_VERSIONS).join(", ")}` ); } // Then check if it's supported for this specific API - const supportedVersions = - DeepgramClient.SUPPORTED_VERSIONS[api as keyof typeof DeepgramClient.SUPPORTED_VERSIONS]; - if (!supportedVersions.includes(version)) { + const supportedVersions = DeepgramClient.SUPPORTED_VERSIONS[api as keyof typeof DeepgramClient.SUPPORTED_VERSIONS]; + if (!supportedVersions) { + throw new DeepgramError(`Unknown API namespace '${api}'.`); + } + if (!supportedVersions.includes(version)) { throw new DeepgramError( `Version '${version}' is not supported for ${api} API. Supported versions: ${supportedVersions.join( ", " )}` ); } }src/sdk/ListenClient.ts (3)
25-36: Prefer delegating tosuper.live(...)to avoid future divergenceReimplementing the construction here duplicates core logic and risks drift if core behavior changes (e.g., connection setup, defaults). Delegating preserves single source of truth.
- // Create the core live client - const coreClient = new CoreListenLiveClient(this.options, transcriptionOptions, endpoint); + // Create the core live client using the core implementation + const coreClient = super.live(transcriptionOptions, endpoint) as CoreListenLiveClient;
14-16: RedundantprerecordedoverrideCore already provides this identical getter. Keeping it overridden increases maintenance burden with no added value.
If there’s no SDK-specific behavior needed, remove the override:
- get prerecorded() { - return new ListenRestClient(this.options); - } + // Inherit `prerecorded` getter from core
44-52: Make v2 detection more robust for custom endpoints
endsWith("/v2/listen")can miss valid variants (e.g., without a leading slash, extra base path). Consider normalizing the path or using a regex to detect segment-wise.- const isV2 = this.version === "v2" || resolvedEndpoint.endsWith("/v2/listen"); + const normalized = resolvedEndpoint.replace(/^\/+/, ""); + const isV2 = + this.version === "v2" || /(^|\/)v2\/listen$/.test(normalized);tests/unit/sdk-enhanced-features.test.ts (3)
13-15: Avoid leaking the WebSocket mock across test suitesOverwriting the global WebSocket without restoring it can affect other tests. Save the original and restore in
afterAll.-// Mock the WebSocket constructor -(global as any).WebSocket = jest.fn(() => mockWebSocket); +// Mock the WebSocket constructor (restore after suite) +const OriginalWebSocket = (global as any).WebSocket; +beforeAll(() => { + (global as any).WebSocket = jest.fn(() => mockWebSocket); +}); +afterAll(() => { + (global as any).WebSocket = OriginalWebSocket; +});
24-32: Use event constants to avoid string driftUsing
LiveTranscriptionEvents.Transcriptinstead of"Results"prevents typos and keeps tests aligned with core enums.- ListenV2.use({ - event: "Results", + ListenV2.use({ + event: LiveTranscriptionEvents.Transcript, before: (payload) => { payload.enhanced = true; }, after: (payload) => { console.log("Processed:", payload.enhanced); }, });
89-93: Prefer constants for event names in instance middlewareSame rationale: guard against typos and event renames.
- (connection as any).use({ - event: "SpeechStarted", + (connection as any).use({ + event: LiveTranscriptionEvents.SpeechStarted, before: () => console.log("User started speaking"), });src/sdk/middleware/types.ts (1)
26-33: Tighten event typing to catch mistakes at compile time
event: stringis permissive. You can constrain it to the known live event values while still allowing custom events if needed.+import type { LiveTranscriptionEvents } from "../../core/lib/enums/LiveTranscriptionEvents"; + +type KnownEvent = (typeof LiveTranscriptionEvents)[keyof typeof LiveTranscriptionEvents]; + export interface ListenV2Middleware { /** The event name to attach to */ - event: string; + event: KnownEvent | string;demo-enhanced-sdk.js (3)
4-4: Remove unused import and silencerequirerule for this demo script
LiveTranscriptionEventsisn't used; also, ESLint flagsrequire()in this repo. Either convert to ESM or disable the rule for this file since this is a Node demo.+/* eslint-disable @typescript-eslint/no-require-imports */ // Demo script showing the enhanced SDK features -const { createClient, ListenV2, LiveTranscriptionEvents } = require("./dist/main/index"); +const { createClient, ListenV2 } = require("./dist/main/index");
44-48: Defensive error formatting in demo
e.messagemay be undefined (or not a string). Avoid throwing in the catch block.- console.log( - " ⚠️ v1 connection failed (expected without API key):", - e.message.substring(0, 50) + "..." - ); + const msg = e && typeof e.message === "string" ? e.message : String(e); + console.log(" ⚠️ v1 connection failed (expected without API key):", msg.slice(0, 50) + "...");
85-89: Defensive error formatting in demo (v2 case)Same improvement for the v2 catch block.
- console.log( - " ⚠️ v2 connection failed (expected without API key):", - e.message.substring(0, 50) + "..." - ); + const msg = e && typeof e.message === "string" ? e.message : String(e); + console.log(" ⚠️ v2 connection failed (expected without API key):", msg.slice(0, 50) + "...");src/sdk/listen/ListenV2Supervisor.ts (2)
139-157: Remove unused parameter and dead path after refactorAfter wrapping
on,executeMiddlewaresis no longer needed. Alternatively, if you keep it, drop the unusedargsparameter to satisfy lint.Option A (remove function entirely):
- /** - * Execute middlewares asynchronously without blocking event emission - */ - private static async executeMiddlewares( - session: AbstractLiveClient, - event: string, - payload: any, - ctx: MiddlewareContext, - originalEmit: Function, - args: any[] - ): Promise<void> { - try { - // Execute before middlewares - await this.registry.executeBefore(session, event, payload, ctx); - - // After middlewares run after the original event has been processed - await this.registry.executeAfter(session, event, payload, ctx); - } catch (error) { - // Emit middleware error but don't break the event chain - originalEmit("middleware_error", { event, error, payload }); - } - }Option B (keep function but fix lint):
- args: any[] + /* unused */ _args?: any[]
91-93: Cleanup: clear plugin references on sessionAvoid lingering references to plugins after disconnect to help GC.
// Store plugins for cleanup - (session as any)._plugins = [turnCountingPlugin, reconnectionPlugin]; + (session as any)._plugins = [turnCountingPlugin, reconnectionPlugin]; + // Optional: also clear on "close" in case user doesn't call disconnect + session.on?.("close", () => { + (session as any)._plugins = undefined; + });src/sdk/middleware/registry.ts (3)
68-80: Run "after" hooks in reverse order to preserve middleware stack semanticsTypical middleware stacks execute "before" in registration order and "after" in reverse (LIFO). Executing both in the same order can make composed middlewares harder to reason about.
Apply this diff:
async executeAfter( session: AbstractLiveClient, event: string, payload: any, ctx: MiddlewareContext ): Promise<void> { - const middlewares = this.getMiddlewares(session, event); - for (const middleware of middlewares) { + const middlewares = this.getMiddlewares(session, event); + for (const middleware of middlewares.slice().reverse()) { if (middleware.after) { await middleware.after(payload, ctx); } } }
12-12: Use WeakMap to prevent instance-level memory leaksIf a session is GC’ed without an explicit dispose(), the Map will retain it. WeakMap ensures we don’t keep strong references to sessions.
- private instanceMiddlewares = new Map<AbstractLiveClient, ListenV2Middleware[]>(); + private instanceMiddlewares = new WeakMap<AbstractLiveClient, ListenV2Middleware[]>();
17-19: Provide a way to clear/remove global middlewares for test isolationStatic global state can bleed across tests and usages. Add a remover/reset API.
static use(middleware: ListenV2Middleware): void { this.globalMiddlewares.push(middleware); } + + /** + * Remove a previously-registered global middleware + */ + static remove(middleware: ListenV2Middleware): void { + this.globalMiddlewares = this.globalMiddlewares.filter((mw) => mw !== middleware); + } + + /** + * Clear all global middlewares (useful for tests) + */ + static clearGlobal(): void { + this.globalMiddlewares = []; + }src/sdk/middleware/plugins.ts (1)
26-53: Option fields userEndEvent/agentEndEvent are unused; either remove or implement for clarityCurrently you only listen to start events. If end events are not needed, drop them from options to avoid confusion. Alternatively, reset speaker state on end events.
Example addition to reset speaker state:
// Store turn count on session for access (session as any)._turnCount = () => turnCount; (session as any)._currentSpeaker = () => currentSpeaker; + + // Optionally reset speaker on end events for clarity + if (opts.userEndEvent) { + api.on(opts.userEndEvent, () => { + if (currentSpeaker === "user") currentSpeaker = null; + }); + } + if (opts.agentEndEvent) { + api.on(opts.agentEndEvent, () => { + if (currentSpeaker === "agent") currentSpeaker = null; + }); + }src/sdk/index.ts (2)
18-22: Fix JSDoc: @param docs don’t match the overloaded signatureAvoid mentioning a non-existent DeepgramClientArgs parameter.
/** * Creates a new Deepgram client instance with enhanced SDK features. * - * @param {DeepgramClientArgs} args - Arguments to pass to the Deepgram client constructor. - * @returns A new enhanced Deepgram client instance. + * Overloads: + * - createClient() + * - createClient(key: string | IKeyFactory) + * - createClient(options: DeepgramClientOptions) + * - createClient(key: string | IKeyFactory, options: DeepgramClientOptions) + * @returns {DeepgramClient} A new enhanced Deepgram client instance. */
33-41: Guard against null when treating keyOrOptions as an objecttypeof null === "object". This makes null fall into the options branch. Add a null check.
- } else if (typeof keyOrOptions === "object") { + } else if (keyOrOptions && typeof keyOrOptions === "object") { resolvedOptions = keyOrOptions; }
| console.log("\n4️⃣ Testing invalid version (should fail)..."); | ||
| try { | ||
| const invalidClient = deepgram.v("v3"); | ||
| const listenClient = invalidClient.listen; // Try to access any API | ||
| console.log(" ❌ v3 should have failed but didn't"); | ||
| } catch (error) { | ||
| if (error.message.includes("not supported")) { | ||
| console.log(" ✅ v3 correctly blocked:", error.message.split(".")[0]); |
There was a problem hiding this comment.
Fix ESLint error: remove unused variable listenClient (breaks CI).
'listenClient' is assigned a value but never used at Line 77. You just need to access the property to trigger validation.
Apply this diff:
- try {
- const invalidClient = deepgram.v("v3");
- const listenClient = invalidClient.listen; // Try to access any API
- console.log(" ❌ v3 should have failed but didn't");
- } catch (error) {
+ try {
+ // Access any API to trigger version validation
+ // eslint-disable-next-line no-unused-expressions
+ deepgram.v("v3").listen;
+ console.log(" ❌ v3 should have failed but didn't");
+ } catch (error) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log("\n4️⃣ Testing invalid version (should fail)..."); | |
| try { | |
| const invalidClient = deepgram.v("v3"); | |
| const listenClient = invalidClient.listen; // Try to access any API | |
| console.log(" ❌ v3 should have failed but didn't"); | |
| } catch (error) { | |
| if (error.message.includes("not supported")) { | |
| console.log(" ✅ v3 correctly blocked:", error.message.split(".")[0]); | |
| console.log("\n4️⃣ Testing invalid version (should fail)..."); | |
| try { | |
| // Access any API to trigger version validation | |
| // eslint-disable-next-line no-unused-expressions | |
| deepgram.v("v3").listen; | |
| console.log(" ❌ v3 should have failed but didn't"); | |
| } catch (error) { | |
| if (error.message.includes("not supported")) { | |
| console.log(" ✅ v3 correctly blocked:", error.message.split(".")[0]); |
🧰 Tools
🪛 GitHub Check: Test / OS ubuntu-latest / Node 22
[failure] 77-77:
'listenClient' is assigned a value but never used
[failure] 77-77:
'listenClient' is assigned a value but never used
🪛 ESLint
[error] 77-77: 'listenClient' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Actions: CI
[error] 77-77: ESLint: 'listenClient' is assigned a value but never used. (no-unused-vars)
🤖 Prompt for AI Agents
In examples/test-version-validation/index.js around lines 74 to 81, ESLint
complains that the variable listenClient is assigned but never used; to fix,
remove the unused variable assignment and just access the property to trigger
validation (e.g., replace the assignment with a direct property access
expression or prefix with void to avoid creating a variable), keeping the
try/catch logic intact so the validation still throws as expected.
src/sdk/listen/ListenV2Supervisor.ts
Outdated
| private static attachBuiltinPlugins(session: AbstractLiveClient): void { | ||
| const api: SessionPluginAPI = { | ||
| on: (event: string, handler: Function) => | ||
| session.on(event, handler as (...args: any[]) => void), | ||
| emit: (event: string, data?: any) => session.emit(event, data), | ||
| getOptions: () => (session as any).options || {}, | ||
| setReconnectOptions: (updater: (options: any) => any) => { | ||
| // Store the updater function for use during reconnection | ||
| (session as any)._reconnectOptionsUpdater = updater; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check that _reconnectOptionsUpdater is honored by core
If core’s AbstractLiveClient.reconnect doesn’t consume _reconnectOptionsUpdater, the overrides won’t be applied. Verify usage across the codebase.
Run this script to search for usages and confirm application:
🏁 Script executed:
#!/bin/bash
# Find where _reconnectOptionsUpdater is referenced and how reconnect is implemented
rg -n -C3 '_reconnectOptionsUpdater'
rg -n -C5 '\breconnect\s*\('Length of output: 1394
🏁 Script executed:
#!/bin/bash
# Locate where AbstractLiveClient is defined
file=$(rg -l 'class AbstractLiveClient')
echo "AbstractLiveClient defined in: $file"
# Inspect the reconnect method implementation
echo -e "\n--- reconnect method ---"
rg -n -C3 'reconnect\s*\(' "$file"
# Check for any usage of _reconnectOptionsUpdater inside that method
echo -e "\n--- usage of _reconnectOptionsUpdater ---"
rg -n -C3 '_reconnectOptionsUpdater' "$file"Length of output: 543
Enforce _reconnectOptionsUpdater in AbstractLiveClient.reconnect
The reconnect implementation in src/core/packages/AbstractLiveClient.ts never applies the stored _reconnectOptionsUpdater, so any overrides passed via setReconnectOptions will be ignored. Please update the method to invoke (and clear) the updater before proceeding.
• File to update: src/core/packages/AbstractLiveClient.ts
• Method: async reconnect(...)
Suggested patch:
--- a/src/core/packages/AbstractLiveClient.ts
+++ b/src/core/packages/AbstractLiveClient.ts
@@ class AbstractLiveClient {
- async reconnect(options: ConnectOptions): Promise<void> {
+ async reconnect(options: ConnectOptions): Promise<void> {
+ // Apply any queued overrides from setReconnectOptions
+ if ((this as any)._reconnectOptionsUpdater) {
+ options = (this as any)._reconnectOptionsUpdater(options);
+ delete (this as any)._reconnectOptionsUpdater;
+ }
// existing reconnect logic...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static attachBuiltinPlugins(session: AbstractLiveClient): void { | |
| const api: SessionPluginAPI = { | |
| on: (event: string, handler: Function) => | |
| session.on(event, handler as (...args: any[]) => void), | |
| emit: (event: string, data?: any) => session.emit(event, data), | |
| getOptions: () => (session as any).options || {}, | |
| setReconnectOptions: (updater: (options: any) => any) => { | |
| // Store the updater function for use during reconnection | |
| (session as any)._reconnectOptionsUpdater = updater; | |
| }, | |
| }; | |
| async reconnect(options: ConnectOptions): Promise<void> { | |
| // Apply any queued overrides from setReconnectOptions | |
| if ((this as any)._reconnectOptionsUpdater) { | |
| options = (this as any)._reconnectOptionsUpdater(options); | |
| delete (this as any)._reconnectOptionsUpdater; | |
| } | |
| // existing reconnect logic... | |
| } |
🤖 Prompt for AI Agents
In src/core/packages/AbstractLiveClient.ts inside the async reconnect(...)
method, the stored (session) property _reconnectOptionsUpdater is never applied
so setReconnectOptions calls are ignored; before performing the reconnect logic,
check if (this as any)._reconnectOptionsUpdater is a function, call it with the
current reconnect options to produce updated options, assign the result back to
the reconnect options used by reconnect, and then delete or set (this as
any)._reconnectOptionsUpdater = undefined to clear it so it won't be reused on
subsequent reconnects; ensure this happens synchronously at the start of
reconnect and that updated options are validated/merged with defaults as the
original flow expects.
src/sdk/listen/ListenV2Supervisor.ts
Outdated
| getOptions: () => (session as any).options || {}, | ||
| setReconnectOptions: (updater: (options: any) => any) => { | ||
| // Store the updater function for use during reconnection | ||
| (session as any)._reconnectOptionsUpdater = updater; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure reconnection uses updated options
setReconnectOptions only stores an updater; getOptions returns the original options. Apply the updater when fetching options so reconnections include headers/query overrides.
- getOptions: () => (session as any).options || {},
- setReconnectOptions: (updater: (options: any) => any) => {
- // Store the updater function for use during reconnection
- (session as any)._reconnectOptionsUpdater = updater;
- },
+ getOptions: () => {
+ const base = ((session as any).options || {});
+ const updater = (session as any)._reconnectOptionsUpdater;
+ return typeof updater === "function" ? updater(base) : base;
+ },
+ setReconnectOptions: (updater: (options: any) => any) => {
+ (session as any)._reconnectOptionsUpdater = updater;
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getOptions: () => (session as any).options || {}, | |
| setReconnectOptions: (updater: (options: any) => any) => { | |
| // Store the updater function for use during reconnection | |
| (session as any)._reconnectOptionsUpdater = updater; | |
| }, | |
| }; | |
| getOptions: () => { | |
| const base = (session as any).options || {}; | |
| const updater = (session as any)._reconnectOptionsUpdater; | |
| return typeof updater === "function" ? updater(base) : base; | |
| }, | |
| setReconnectOptions: (updater: (options: any) => any) => { | |
| (session as any)._reconnectOptionsUpdater = updater; | |
| }, |
🤖 Prompt for AI Agents
In src/sdk/listen/ListenV2Supervisor.ts around lines 56-61, the bug is that
setReconnectOptions only stores an updater function but getOptions always
returns the original session options; change getOptions so it checks (session as
any)._reconnectOptionsUpdater and if present calls it with the current options,
uses the updater's return as the new effective options (and assign it back to
(session as any).options so future calls see the update), and return that
updated options; ensure it handles a missing updater or updater that returns
undefined by falling back to the original options.
src/sdk/listen/ListenV2Supervisor.ts
Outdated
| private static wrapEventEmission(session: AbstractLiveClient): void { | ||
| const originalEmit = session.emit.bind(session); | ||
|
|
||
| (session as any).emit = (event: string, ...args: any[]) => { | ||
| const payload = args[0]; | ||
| const ctx: MiddlewareContext = { | ||
| session, | ||
| turnCount: (session as any)._turnCount?.() || 0, | ||
| connectionAttempt: (session as any)._connectionAttempt || 0, | ||
| metadata: (session as any)._lastMetadata, | ||
| resumeContext: (session as any)._resumeContext, | ||
| }; | ||
|
|
||
| // Execute middlewares asynchronously but don't block the event emission | ||
| this.executeMiddlewares(session, event, payload, ctx, originalEmit, args); | ||
|
|
||
| // Return the original emission result immediately | ||
| return originalEmit(event, ...args); | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Middleware ordering mismatch: before runs after emission
By wrapping emit and scheduling async work, before middlewares may execute after user handlers have already run, defeating the purpose of “before”. Wrap on-registered handlers instead so you can run before -> user handler -> after deterministically without changing emit’s sync contract.
Apply this diff to wrap handlers (and drop the current emit wrapper):
- /**
- * Wrap the session's emit method to run middlewares
- */
- private static wrapEventEmission(session: AbstractLiveClient): void {
- const originalEmit = session.emit.bind(session);
-
- (session as any).emit = (event: string, ...args: any[]) => {
- const payload = args[0];
- const ctx: MiddlewareContext = {
- session,
- turnCount: (session as any)._turnCount?.() || 0,
- connectionAttempt: (session as any)._connectionAttempt || 0,
- metadata: (session as any)._lastMetadata,
- resumeContext: (session as any)._resumeContext,
- };
-
- // Execute middlewares asynchronously but don't block the event emission
- this.executeMiddlewares(session, event, payload, ctx, originalEmit, args);
-
- // Return the original emission result immediately
- return originalEmit(event, ...args);
- };
- }
+ /**
+ * Wrap the session's `on` to run middlewares: before -> handler -> after
+ */
+ private static wrapEventEmission(session: AbstractLiveClient): void {
+ const originalOn = session.on.bind(session);
+ (session as any).on = ((event: string, handler: (...args: any[]) => void) => {
+ const wrapped = async (...args: any[]) => {
+ const payload = args[0];
+ const ctx: MiddlewareContext = {
+ session,
+ turnCount: (session as any)._turnCount?.() || 0,
+ connectionAttempt: (session as any)._connectionAttempt || 0,
+ metadata: (session as any)._lastMetadata,
+ resumeContext: (session as any)._resumeContext,
+ };
+ try {
+ await this.registry.executeBefore(session, event, payload, ctx);
+ handler(...args);
+ await this.registry.executeAfter(session, event, payload, ctx);
+ } catch (error) {
+ session.emit("middleware_error", { event, error, payload });
+ }
+ };
+ return originalOn(event, wrapped as (...args: any[]) => void);
+ }) as any;
+ }Committable suggestion skipped: line range outside the PR's diff.
src/sdk/ListenClient.ts
Outdated
| // Debug logging for tests | ||
| if (process.env.NODE_ENV === "test") { | ||
| console.log( | ||
| `shouldSupervise: version=${this.version}, endpoint=${endpoint}, resolved=${resolvedEndpoint}, isV2=${isV2}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against process being undefined in browsers
Referencing process.env directly can throw in browser builds where process is not defined.
Apply this diff to make the check safe across environments:
- if (process.env.NODE_ENV === "test") {
+ if (typeof process !== "undefined" && process?.env?.NODE_ENV === "test") {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Debug logging for tests | |
| if (process.env.NODE_ENV === "test") { | |
| console.log( | |
| `shouldSupervise: version=${this.version}, endpoint=${endpoint}, resolved=${resolvedEndpoint}, isV2=${isV2}` | |
| ); | |
| } | |
| // Debug logging for tests | |
| if (typeof process !== "undefined" && process?.env?.NODE_ENV === "test") { | |
| console.log( | |
| `shouldSupervise: version=${this.version}, endpoint=${endpoint}, resolved=${resolvedEndpoint}, isV2=${isV2}` | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/sdk/ListenClient.ts around lines 53 to 58, the debug logging uses
process.env directly which can throw in browser builds where process is
undefined; update the condition to first check that typeof process !==
"undefined" (and optionally process.env exists) before comparing NODE_ENV to
"test", then keep the console.log call inside that guarded block so the log only
runs in safe environments.
| import { ListenLiveClient } from "../../src/core/packages/ListenLiveClient"; | ||
| import { LiveTranscriptionEvents } from "../../src/core/lib/enums/LiveTranscriptionEvents"; | ||
| import { CONNECTION_STATE } from "../../src/core/lib/constants"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Import path migration looks good; consider de-coupling tests from internal paths via a core barrel import.
These deep relative paths tie tests to internal folder structure. If src/core/index.ts re-exports these, prefer importing from the barrel to reduce churn when moving files.
If the barrel re-exports are available, you can simplify as:
-import { ListenLiveClient } from "../../src/core/packages/ListenLiveClient";
-import { LiveTranscriptionEvents } from "../../src/core/lib/enums/LiveTranscriptionEvents";
-import { CONNECTION_STATE } from "../../src/core/lib/constants";
+import { ListenLiveClient, LiveTranscriptionEvents, CONNECTION_STATE } from "../../src/core";Run this script to verify the re-exports exist:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify core barrel file exists
fd -a --max-depth 2 '^index\.ts$' src/core | tee /tmp/core_index_files.txt
if [[ ! -s /tmp/core_index_files.txt ]]; then
echo "No src/core/index.ts barrel found."
exit 1
fi
idx="$(head -n1 /tmp/core_index_files.txt)"
echo "Checking barrel exports in: $idx"
# Check for the three symbols
for sym in ListenLiveClient LiveTranscriptionEvents CONNECTION_STATE; do
if ! rg -nP "\bexport\b.*\b${sym}\b" "$idx"; then
echo "Missing re-export for ${sym} in $idx"
fi
done
# Fallback: ensure symbols are exported somewhere in src/core
for sym in ListenLiveClient LiveTranscriptionEvents CONNECTION_STATE; do
echo "Searching for export of ${sym} in src/core..."
rg -nP --type=ts -C2 "(export\s+(class|const|enum|type|interface)\s+${sym}\b)|(export\s*\{[^}]*\b${sym}\b[^}]*\})" src/core || true
doneLength of output: 190
I’ve updated the verification script to avoid the unavailable tee command and directly locate and inspect the barrel. Please run this:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate any src/core/index.ts barrel files
barrels=$(fd --max-depth 2 -a '^index\.ts$' src/core)
if [[ -z "$barrels" ]]; then
echo "No src/core/index.ts barrel found."
exit 1
fi
# Check each barrel for the three exports
for barrel in $barrels; do
echo "Checking exports in $barrel"
rg -nP "\bexport\b.*\bListenLiveClient\b" "$barrel" || echo "→ Missing re-export: ListenLiveClient"
rg -nP "\bexport\b.*\bLiveTranscriptionEvents\b" "$barrel" || echo "→ Missing re-export: LiveTranscriptionEvents"
rg -nP "\bexport\b.*\bCONNECTION_STATE\b" "$barrel" || echo "→ Missing re-export: CONNECTION_STATE"
doneLength of output: 1586
Missing barrel exports for core imports
The root barrel (src/core/index.ts) doesn’t re-export the symbols your tests need (only ListenLiveClient is in src/core/packages/index.ts; the others aren’t exposed). Without adding them, switching tests to import { … } from "src/core" will break.
Please update your barrel (e.g. src/core/index.ts) to include:
export { ListenLiveClient } from "./packages/ListenLiveClient";
export { LiveTranscriptionEvents } from "./lib/enums/LiveTranscriptionEvents";
export { CONNECTION_STATE } from "./lib/constants";After that, you can simplify your test imports as follows:
-import { ListenLiveClient } from "../../src/core/packages/ListenLiveClient";
-import { LiveTranscriptionEvents } from "../../src/core/lib/enums/LiveTranscriptionEvents";
-import { CONNECTION_STATE } from "../../src/core/lib/constants";
+import { ListenLiveClient, LiveTranscriptionEvents, CONNECTION_STATE } from "../../src/core";🤖 Prompt for AI Agents
In tests/e2e/listen-live-connection.test.ts lines 1-3: the test imports three
symbols directly from deep paths but the core root barrel (src/core/index.ts)
does not re-export LiveTranscriptionEvents and CONNECTION_STATE, so add exports
for ListenLiveClient, LiveTranscriptionEvents, and CONNECTION_STATE to
src/core/index.ts (pointing to ./packages/ListenLiveClient,
./lib/enums/LiveTranscriptionEvents, and ./lib/constants respectively), then
update the test imports to import those symbols from "src/core" instead of deep
paths.
| import { structureOnlySerializer, setupApiMocks, cleanupApiMocks } from "../__utils__"; | ||
| import { testProjectIds } from "../__fixtures__/manage"; | ||
| import type { DeepgramResponse, GetProjectResponse } from "../../src/lib/types"; | ||
| import type { DeepgramResponse, GetProjectResponse } from "../../src/core/lib/types"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Switched to core types import; consider sourcing from SDK or core barrel for a stable public surface
Using "../../src/core/lib/types" is aligned with the split. For public-facing tests, prefer:
- Re-exported types from the SDK root (../../src/index) if they’re part of the public API, or
- The core barrel (../../src/core) if SDK intentionally does not expose them.
This reduces churn from internal directory changes.
If types are re-exported from core:
-import type { DeepgramResponse, GetProjectResponse } from "../../src/core/lib/types";
+import type { DeepgramResponse, GetProjectResponse } from "../../src/core";Validate there are no lingering imports from the old location and that core index exposes these types:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Ensure no imports from ../../src/lib/types remain..."
rg -nP 'from\s+["\']\.\.\/\.\.\/src\/lib\/types["\']' tests || true
echo "Confirm DeepgramResponse and GetProjectResponse are exported in core..."
rg -nP '\bexport\s+(type|interface)\s+DeepgramResponse\b' src/core || true
rg -nP '\bexport\s+(type|interface)\s+GetProjectResponse\b' src/core || trueLength of output: 251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "🔍 Searching for imports from src/core/lib/types in tests..."
rg -n "../../src/core/lib/types" tests || echo "✅ No imports from src/core/lib/types found."
echo ""
echo "🔍 Checking for re-exports from lib/types in core barrel (src/core/index.ts)..."
rg -n "export .*from ['\"]\\./lib/types['\"]" src/core/index.ts || echo "⚠️ No direct re-export from lib/types in src/core/index.ts."
echo ""
echo "🔍 Confirming DeepgramResponse and GetProjectResponse appear in core barrel..."
rg -n "DeepgramResponse" src/core/index.ts || echo "⚠️ DeepgramResponse not found in src/core/index.ts."
rg -n "GetProjectResponse" src/core/index.ts || echo "⚠️ GetProjectResponse not found in src/core/index.ts."Length of output: 4307
Update E2E tests to import types from the core barrel
Most E2E tests still pull types directly from the internal src/core/lib/types. Since src/core/index.ts already re-exports everything via
export * from "./lib/types";you should update all test and mock files to use the public barrel path instead:
• tests/e2e/**/.test.ts importing from "../../src/core/lib/types"
• tests/e2e/mocks/.ts importing from "../../../src/core/lib/types"
Example change:
-import type { DeepgramResponse, GetProjectResponse } from "../../src/core/lib/types";
+import type { DeepgramResponse, GetProjectResponse } from "../../src/core";🤖 Prompt for AI Agents
In tests/e2e/manage-get-project.test.ts around line 4, the test imports types
directly from the internal path "../../src/core/lib/types"; change the import to
use the public core barrel path ("../../src/core") instead. Update the import
statement to reference the barrel so the test consumes exported types via
src/core index, and run tests to ensure no import path errors remain.
| import { createClient } from "../../src/index"; | ||
| import { structureOnlySerializer, setupApiMocks, cleanupApiMocks } from "../__utils__"; | ||
| import type { DeepgramResponse, GetProjectsResponse } from "../../src/lib/types"; | ||
| import type { DeepgramResponse, GetProjectsResponse } from "../../src/core/lib/types"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid importing types from core in E2E tests; prefer SDK entrypoint re-exports.
To prevent leaking internal layering details into tests and reduce churn from internal reorganizations, consider re-exporting these types from the SDK entrypoint and importing them from there in tests.
If/when you re-export types from the SDK entrypoint (e.g., src/index.ts), add:
// src/index.ts
export type { DeepgramResponse, GetProjectsResponse } from "./core/lib/types";Then adjust this test import:
-import type { DeepgramResponse, GetProjectsResponse } from "../../src/core/lib/types";
+import type { DeepgramResponse, GetProjectsResponse } from "../../src/index";You can locate all tests still importing types from core with:
🏁 Script executed:
#!/bin/bash
# Find all direct imports of core/lib/types across tests
rg -nP --type=ts -C1 'from\s+[\'"][\.\/]*src/core/lib/types[\'"]' testsLength of output: 116
🏁 Script executed:
#!/bin/bash
# Retry finding direct imports of core/lib/types in tests with corrected quoting
rg -nP --type=ts -C1 "from\s+['\"][./]*src/core/lib/types['\"]" testsLength of output: 7706
Avoid direct core/lib/types imports in E2E tests
We’ve identified numerous E2E tests and mocks still pulling types from src/core/lib/types, which couples tests to our internal module structure. To stabilize test code against future refactorings:
-
In your SDK entrypoint (
src/index.ts), re-export all needed types:// src/index.ts export type { DeepgramResponse, CreateProjectKeyResponse, GetProjectsResponse, GetProjectInvitesResponse, GetProjectBalancesResponse, GetProjectBalanceResponse, GetProjectKeyResponse, MessageResponse, AsyncPrerecordedResponse, AsyncAnalyzeResponse, GetTokenDetailsResponse, GetProjectUsageSummaryResponse, GetProjectUsageRequestResponse, GetProjectUsageFieldsResponse, GetProjectMembersResponse, GetProjectKeysResponse, GetProjectResponse, GetProjectUsageRequestsResponse, GetProjectMemberScopesResponse, VoidResponse, // …and any other types your tests reference } from "./core/lib/types";
-
Update imports in all affected test and mock files. For example:
- import type { DeepgramResponse, GetProjectsResponse } from "../../src/core/lib/types"; + import type { DeepgramResponse, GetProjectsResponse } from "../../src";
-
Affected locations include (but are not limited to):
• tests/e2e/manage-create-project-key.test.ts
• tests/e2e/manage-get-project-invites.test.ts
• tests/e2e/manage-get-project-balances.test.ts
• tests/e2e/manage-get-project-balance.test.ts
• tests/e2e/manage-get-project-key.test.ts
• tests/e2e/manage-send-project-invite.test.ts
• tests/e2e/listen-transcribe-url-callback.test.ts
• tests/e2e/read-analyze-url-callback.test.ts
• tests/e2e/read-analyze-text-callback.test.ts
• tests/e2e/manage-update-project.test.ts
• tests/e2e/manage-get-token-details.test.ts
• tests/e2e/manage-update-project-member-scope.test.ts
• tests/e2e/manage-get-project-usage-summary.test.ts
• tests/e2e/manage-get-project-usage-request.test.ts
• tests/e2e/manage-get-project-usage-fields.test.ts
• tests/e2e/manage-get-project-members.test.ts
• tests/e2e/manage-get-project-keys.test.ts
• tests/e2e/listen-transcribe-file-callback.test.ts
• tests/e2e/manage-get-project.test.ts
• tests/e2e/manage-get-project-usage-requests.test.ts
• tests/e2e/mocks/read.ts
• tests/e2e/mocks/manage.ts
• tests/e2e/mocks/listen.ts
Run this once more to verify no stray imports remain:
rg -lP --type=ts 'from\s+["'\'']\.\.\/\.\.\/src\/core\/lib\/types["'\'']' tests—then update any residual references accordingly.
🤖 Prompt for AI Agents
In tests/e2e/manage-get-projects.test.ts (around line 3) the test imports types
directly from src/core/lib/types which couples tests to internal paths;
re-export the required types from the SDK entrypoint (src/index.ts) and then
change this and all other E2E test/mock files to import those types from the
package root (e.g., from the SDK entrypoint) instead of
../../src/core/lib/types; after updating src/index.ts and the imports, run the
provided ripgrep command to find and fix any remaining direct imports.
| import { SpeakLiveClient } from "../../src/core/packages/SpeakLiveClient"; | ||
| import { LiveTTSEvents } from "../../src/core/lib/enums/LiveTTSEvents"; | ||
| import { CONNECTION_STATE } from "../../src/core/lib/constants"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM on refactor to core paths; verify named exports and file presence
The import updates align with the core/sdk separation. To avoid surprises (default vs named exports, missing files), run the quick checks below.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect these files to exist at repo root paths (tests import via ../../src/core/...)
paths=(
"src/core/packages/SpeakLiveClient.ts"
"src/core/lib/enums/LiveTTSEvents.ts"
"src/core/lib/constants.ts"
)
for p in "${paths[@]}"; do
if [ ! -f "$p" ]; then
echo "Missing file: $p" >&2
exit 1
fi
done
echo "Files exist. Verifying named exports…"
# 1) SpeakLiveClient should be a named (not default) export
rg -nP -C2 --fixed-strings "export class SpeakLiveClient" src/core/packages/SpeakLiveClient.ts || {
echo "SpeakLiveClient not exported as a named class. Checking for default export…" >&2
if rg -nP -C2 "export\s+default\s+class\s+SpeakLiveClient\b" src/core/packages/SpeakLiveClient.ts; then
echo "Found default export for SpeakLiveClient; tests use a named import which will fail." >&2
exit 2
else
echo "Could not find SpeakLiveClient export." >&2
exit 3
fi
}
# 2) LiveTTSEvents should be exported (enum or const)
if ! rg -nP -C2 "export\s+(enum|const)\s+LiveTTSEvents\b" src/core/lib/enums/LiveTTSEvents.ts; then
echo "LiveTTSEvents not exported as enum/const from src/core/lib/enums/LiveTTSEvents.ts" >&2
exit 4
fi
# 3) CONNECTION_STATE should be exported (enum or const)
if ! rg -nP -C2 "export\s+(enum|const)\s+CONNECTION_STATE\b" src/core/lib/constants.ts; then
echo "CONNECTION_STATE not exported as enum/const from src/core/lib/constants.ts" >&2
exit 5
fi
# 4) Validate SpeakLiveClient methods referenced in tests exist
required_methods=(connectionState disconnect sendText flush clear requestClose)
for m in "${required_methods[@]}"; do
if ! rg -nPU "(?s)^\s*(?:export\s+)?(?:default\s+)?class\s+SpeakLiveClient\b.*?\b${m}\s*\(" src/core/packages/SpeakLiveClient.ts > /dev/null; then
echo "Method '${m}' not found on SpeakLiveClient (or not within the class body)." >&2
exit 6
fi
done
echo "All checks passed."Length of output: 1597
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any mention of 'connectionState' in SpeakLiveClient
rg -n "connectionState" -C3 src/core/packages/SpeakLiveClient.ts || echo "No 'connectionState' occurrences found"Length of output: 177
Missing connectionState on SpeakLiveClient – tests will fail
The E2E test references a connectionState member on SpeakLiveClient, but no such method or property exists in src/core/packages/SpeakLiveClient.ts. You’ll need to either:
- Implement a
connectionStategetter or method onSpeakLiveClient, or - Update the test to use the correct API (e.g. an existing method/property that returns the client’s state).
Locations to address:
- tests/e2e/speak-live-connection.test.ts (calls
client.connectionState) - src/core/packages/SpeakLiveClient.ts (add or rename the member)
🤖 Prompt for AI Agents
In tests/e2e/speak-live-connection.test.ts around lines 1-3 the test calls
client.connectionState but src/core/packages/SpeakLiveClient.ts does not expose
such a member; add a connectionState getter or method to SpeakLiveClient that
returns the current connection state (matching the CONNECTION_STATE enum) or
modify the test to call the existing API that returns the client's state;
implement the getter to read the internal state field (or provide a public
method like getConnectionState()) and ensure return values use the
CONNECTION_STATE constants so the test and implementation agree.
…d supervision - Remove all ListenV2 references from core files - Replace builtinMiddlewares/builtinPlugins with simple middlewares array in SupervisorConfig - Remove unused SessionPlugin and SessionPluginAPI interfaces - Fix Supervisor.use() method to properly use registry instance - Add proper TypeScript types for middleware functions - Remove redundant prerecorded getter override in ListenClient - Remove unused ListenRestClient import - Update tests to match new simplified architecture - All tests passing with cleaner, more maintainable code structure
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
src/sdk/ListenClient.ts (1)
41-49: Gate console logs to safe environments (browser-safe, test/dev only)These logs will fire for every SpeechStarted event, which is noisy and may leak payloads. Also, prior feedback requested guarding process usage for browser builds.
Apply this diff to guard logging:
{ event: "SpeechStarted", before: (payload: any, ctx: MiddlewareContext) => { - console.log("before:SpeechStarted", payload, ctx); + if (typeof process !== "undefined" && process?.env?.NODE_ENV === "test") { + console.log("before:SpeechStarted", payload, ctx); + } }, after: (payload: any, ctx: MiddlewareContext) => { - console.log("after:SpeechStarted", payload, ctx); + if (typeof process !== "undefined" && process?.env?.NODE_ENV === "test") { + console.log("after:SpeechStarted", payload, ctx); + } }, },
🧹 Nitpick comments (11)
examples/version-patterns-demo/index.js (2)
9-10: Guard against missing DEEPGRAM_API_KEY for clearer failuresFail fast with a readable error if the environment variable isn’t set. Helps avoid confusing runtime errors against the API.
Apply this diff:
- const deepgram = createClient(process.env.DEEPGRAM_API_KEY); + if (!process.env.DEEPGRAM_API_KEY) { + throw new Error("Set DEEPGRAM_API_KEY in your environment to run this example."); + } + const deepgram = createClient(process.env.DEEPGRAM_API_KEY);
29-31: Avoid relying on private/internal APIs (underscore-prefixed)
_turnCountappears to be an internal helper (underscore prefix). Relying on it in examples risks breakage if internals change. If there’s a public analogue, prefer that; otherwise consider removing these logs from public examples.Also applies to: 62-66, 69-72
examples/central-supervisor-demo/index.js (1)
10-11: Guard against missing DEEPGRAM_API_KEY for clearer failuresAdd a fast-fail guard to make the example friendlier to run.
Apply this diff:
- const deepgram = createClient(process.env.DEEPGRAM_API_KEY); + if (!process.env.DEEPGRAM_API_KEY) { + throw new Error("Set DEEPGRAM_API_KEY in your environment to run this example."); + } + const deepgram = createClient(process.env.DEEPGRAM_API_KEY);examples/node-listen-v2-flux-ready/package.json (1)
1-21: Mark example package as private (avoid accidental publish) and optionally set enginesNot required, but recommended for example apps.
Apply this diff:
{ "name": "node-listen-v2-flux-ready", "version": "1.0.0", "description": "Example demonstrating Listen v2 architecture ready for flux model integration", "main": "index.js", + "private": true, + "engines": { + "node": ">=18" + }, "scripts": { "start": "node index.js" }, "dependencies": { "dotenv": "^16.0.0" }, "keywords": [ "deepgram", "transcription", "websocket", "flux", "middleware" ], "author": "Deepgram", "license": "MIT" }src/sdk/middleware/types.ts (1)
26-33: Consider tightening types (unknown/generics) for payload and metadataTo improve type safety and DX, consider:
- Using
unknownformetadataandpayloadinstead ofany.- Optionally making
Middlewaregeneric:export interface Middleware<TPayload = unknown> { ... }.This is a non-blocking enhancement; fine to defer.
examples/node-listen-v2-flux-ready/README.md (1)
22-25: Polish grammar and punctuation in bulletsMinor editorial tweaks improve readability and address lint hints.
-- Handles global and instance-specific middleware -- Executes before/after hooks for events -- Provides cleanup on session disposal +- Handles global and instance-specific middleware. +- Executes before and after hooks for events. +- Provides cleanup upon session disposal. @@ -- Turn counting with speaker detection -- Reconnection with exponential backoff and jitter -- Resume context preservation +- Turn counting with speaker detection. +- Reconnection with exponential backoff and jitter. +- Resume context preservation.Also applies to: 28-31
src/sdk/ListenClient.ts (2)
56-61: Docstring mismatch: method does not consider endpointThe comment says “based on endpoint and version” but the method only checks version equality. Either include endpoint in the decision or adjust the comment.
- /** - * Determine if a session should be supervised based on endpoint and version - */ + /** + * Determine if a session should be supervised based on version + */
10-10: Optional: expose a convenience global “use” on ListenClient to match examplesExamples use ListenClient.use(...) for global middleware. If you want to keep that DX, add a static proxy to Supervisor.use("listen","v2", …).
export class ListenClient extends CoreListenClient { + /** + * Register a global middleware for Listen v2. + */ + static use(middleware: { event: string; before?: Function; after?: Function }) { + Supervisor.use("listen", "v2", middleware); + }Note: If you adopt this, ensure src/sdk/index.ts exports this ListenClient (see suggestion there).
examples/node-listen-v2-flux-ready/index.js (2)
71-75: Avoid relying on _processed_at in event handlersGiven the supervisor executes middlewares asynchronously and does not block emission, the before hook may not have run before the Transcript listener executes. Guard this usage.
connection.on(LiveTranscriptionEvents.Transcript, (data) => { console.log("📝 Transcript:", data.channel.alternatives[0].transcript); - console.log("⏱️ Processing timestamp:", data._processed_at); + if (data._processed_at !== undefined) { + console.log("⏱️ Processing timestamp:", data._processed_at); + } console.log("🔢 Turn count:", connection._turnCount ? connection._turnCount() : 0); });If you want “before” to reliably run before listeners, we need to change Supervisor.wrapEventEmission semantics (see Supervisor.ts comment).
1-3: ESLint: require() is forbidden—convert example to ESM or add an override for examples/If the repo enforces @typescript-eslint/no-require-imports, either:
- Convert to ESM (import … from …), or
- Add an ESLint override for the examples/ directory to allow require in Node demos.
Do you want me to submit an ESLint override patch for examples/*?
src/sdk/supervisor/Supervisor.ts (1)
186-189: Consider exposing type-safe middleware registrationSupervisor.use(clientType, version, middleware) accepts any. To improve DX and catch typos in event names, consider typing middleware.event to known event unions per clientType/version.
Happy to wire an event-map type so middleware.event is constrained to LiveTranscriptionEvents for listen v2.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
examples/central-supervisor-demo/index.js(1 hunks)examples/node-listen-v2-enhanced/index.js(1 hunks)examples/node-listen-v2-flux-ready/README.md(1 hunks)examples/node-listen-v2-flux-ready/index.js(1 hunks)examples/node-listen-v2-flux-ready/package.json(1 hunks)examples/version-patterns-demo/index.js(1 hunks)examples/version-patterns-demo/package.json(1 hunks)src/sdk/ListenClient.ts(1 hunks)src/sdk/index.ts(1 hunks)src/sdk/middleware/index.ts(1 hunks)src/sdk/middleware/registry.ts(1 hunks)src/sdk/middleware/types.ts(1 hunks)src/sdk/supervisor/Supervisor.ts(1 hunks)src/sdk/supervisor/index.ts(1 hunks)tests/unit/sdk-enhanced-features.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/version-patterns-demo/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/sdk/middleware/index.ts
- examples/node-listen-v2-enhanced/index.js
- src/sdk/middleware/registry.ts
- tests/unit/sdk-enhanced-features.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (6)
examples/node-listen-v2-flux-ready/index.js (3)
src/sdk/ListenClient.ts (1)
ListenClient(10-62)demo-enhanced-sdk.js (1)
processingTime(17-17)src/sdk/index.ts (1)
createClient(46-46)
examples/central-supervisor-demo/index.js (2)
src/sdk/index.ts (2)
createClient(46-46)Supervisor(61-61)src/sdk/supervisor/Supervisor.ts (1)
Supervisor(45-190)
src/sdk/supervisor/Supervisor.ts (5)
src/sdk/index.ts (3)
SupervisorConfig(63-63)MiddlewareContext(62-62)Supervisor(61-61)src/sdk/middleware/types.ts (1)
MiddlewareContext(6-21)src/sdk/middleware/registry.ts (1)
MiddlewareRegistry(7-89)src/core/packages/AbstractLiveClient.ts (1)
AbstractLiveClient(542-542)src/core/lib/version.ts (1)
version(1-1)
src/sdk/middleware/types.ts (2)
src/sdk/index.ts (2)
MiddlewareContext(62-62)Middleware(62-62)src/core/packages/AbstractLiveClient.ts (1)
AbstractLiveClient(542-542)
src/sdk/index.ts (3)
src/core/lib/errors.ts (1)
DeepgramVersionError(42-50)src/sdk/DeepgramClient.ts (1)
DeepgramClient(10-29)src/core/lib/types/DeepgramClientOptions.ts (2)
IKeyFactory(3-3)DeepgramClientOptions(61-88)
src/sdk/ListenClient.ts (6)
src/core/packages/ListenClient.ts (1)
ListenClient(13-35)tests/__fixtures__/listen.ts (1)
transcriptionOptions(40-56)src/sdk/index.ts (3)
SupervisorConfig(63-63)MiddlewareContext(62-62)Supervisor(61-61)src/sdk/supervisor/Supervisor.ts (2)
SupervisorConfig(8-40)Supervisor(45-190)src/sdk/middleware/types.ts (1)
MiddlewareContext(6-21)src/core/lib/version.ts (1)
version(1-1)
🪛 ESLint
examples/version-patterns-demo/index.js
[error] 1-1: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 2-2: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
examples/node-listen-v2-flux-ready/index.js
[error] 1-1: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 2-2: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 7-7: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
examples/central-supervisor-demo/index.js
[error] 1-1: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 2-2: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 25-25: 'agentConfig' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 63-63: 'speakConfig' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 76-76: 'options' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 100-100: 'payload' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: Test / OS ubuntu-latest / Node 22
examples/central-supervisor-demo/index.js
[failure] 100-100:
'payload' is defined but never used
[failure] 100-100:
'payload' is defined but never used. Allowed unused args must match /^_/u
[failure] 76-76:
'options' is assigned a value but never used
[failure] 76-76:
'options' is assigned a value but never used
[failure] 63-63:
'speakConfig' is assigned a value but never used
[failure] 63-63:
'speakConfig' is assigned a value but never used
[failure] 25-25:
'agentConfig' is assigned a value but never used
[failure] 25-25:
'agentConfig' is assigned a value but never used
[failure] 2-2:
A require() style import is forbidden
[failure] 1-1:
A require() style import is forbidden
🪛 GitHub Check: Test / OS ubuntu-latest / Node 18
examples/central-supervisor-demo/index.js
[failure] 100-100:
'payload' is defined but never used
[failure] 100-100:
'payload' is defined but never used. Allowed unused args must match /^_/u
[failure] 76-76:
'options' is assigned a value but never used
[failure] 76-76:
'options' is assigned a value but never used
[failure] 63-63:
'speakConfig' is assigned a value but never used
[failure] 63-63:
'speakConfig' is assigned a value but never used
[failure] 25-25:
'agentConfig' is assigned a value but never used
[failure] 25-25:
'agentConfig' is assigned a value but never used
[failure] 2-2:
A require() style import is forbidden
[failure] 1-1:
A require() style import is forbidden
🪛 GitHub Actions: CI
examples/central-supervisor-demo/index.js
[error] 1-1: ESLint error: @typescript-eslint/no-require-imports. A require() style import is forbidden in /home/runner/work/deepgram-js-sdk/deepgram-js-sdk/examples/central-supervisor-demo/index.js (line 1, column 63). Command: eslint -c .eslintrc-examples.js examples --max-warnings 0
🪛 LanguageTool
examples/node-listen-v2-flux-ready/README.md
[grammar] ~22-~22: There might be a mistake here.
Context: ... global and instance-specific middleware - Executes before/after hooks for events -...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...- Executes before/after hooks for events - Provides cleanup on session disposal ##...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ... - Turn counting with speaker detection - Reconnection with exponential backoff an...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...tion with exponential backoff and jitter - Resume context preservation ## Usage `...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Browser Builds
🔇 Additional comments (4)
src/sdk/supervisor/index.ts (1)
1-1: Barrel export looks goodSimple re-export aligns with the new Supervisor module and keeps the SDK surface tidy.
src/sdk/middleware/types.ts (1)
1-1: Import path and type-only import are appropriateType-only import from core keeps runtime clean and avoids circular deps.
examples/node-listen-v2-flux-ready/README.md (1)
39-43: Clarify event naming to match SDK constantsThe example uses a string "Results". If the runtime uses enumerated LiveTranscriptionEvents (e.g., Transcript, Metadata), consider aligning to avoid confusion.
Would you like me to update the README sample to use LiveTranscriptionEvents.Transcript consistently?
src/sdk/index.ts (1)
11-15: No duplicateDeepgramexport detected
Re-exportingexport * from "../core/packages"(and the other core modules) does not introduce aDeepgramsymbol—no class or named export calledDeepgramexists in the core packages. You can safely keep the SDK’s throwing-constructorDeepgramand leave the star-exports as is.Likely an incorrect or invalid review comment.
| const { createClient, Supervisor, LiveTranscriptionEvents } = require("../../dist/main/index"); | ||
| require("dotenv").config(); |
There was a problem hiding this comment.
Fix ESLint: no-require-imports rule is breaking CI
CommonJS require is flagged by the examples ESLint config and is causing CI failures. Disable the rule at the file level to unblock, or convert to ESM (heavier change across examples).
Apply this diff:
+/* eslint-disable @typescript-eslint/no-require-imports */
const { createClient, Supervisor, LiveTranscriptionEvents } = require("../../dist/main/index");
require("dotenv").config();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { createClient, Supervisor, LiveTranscriptionEvents } = require("../../dist/main/index"); | |
| require("dotenv").config(); | |
| /* eslint-disable @typescript-eslint/no-require-imports */ | |
| const { createClient, Supervisor, LiveTranscriptionEvents } = require("../../dist/main/index"); | |
| require("dotenv").config(); |
🧰 Tools
🪛 ESLint
[error] 1-1: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 2-2: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
🪛 GitHub Check: Test / OS ubuntu-latest / Node 22
[failure] 2-2:
A require() style import is forbidden
[failure] 1-1:
A require() style import is forbidden
🪛 GitHub Check: Test / OS ubuntu-latest / Node 18
[failure] 2-2:
A require() style import is forbidden
[failure] 1-1:
A require() style import is forbidden
🪛 GitHub Actions: CI
[error] 1-1: ESLint error: @typescript-eslint/no-require-imports. A require() style import is forbidden in /home/runner/work/deepgram-js-sdk/deepgram-js-sdk/examples/central-supervisor-demo/index.js (line 1, column 63). Command: eslint -c .eslintrc-examples.js examples --max-warnings 0
🤖 Prompt for AI Agents
In examples/central-supervisor-demo/index.js around lines 1 to 2, ESLint’s
no-require-imports rule is causing CI to fail because the file uses CommonJS
require; add a file-level ESLint directive to disable no-require-imports at the
top of this file (before any require/ imports) so the existing CommonJS code
passes linting, or alternatively convert the file to ESM imports if you prefer a
larger refactor—prefer adding the disable comment for the minimal change to
unblock CI.
| const agentConfig = { | ||
| clientType: "agent", | ||
| version: "v2", | ||
| builtinMiddlewares: [ | ||
| { | ||
| event: "ConversationText", | ||
| before: (payload) => { | ||
| console.log(`🔧 Agent middleware: Processing ${payload.role} message`); | ||
| }, | ||
| }, | ||
| ], | ||
| builtinPlugins: [ | ||
| { | ||
| plugin: (options = {}) => ({ | ||
| attach: (session, api) => { | ||
| console.log(`🔌 Agent plugin attached with options:`, options); | ||
|
|
||
| // Custom agent turn tracking | ||
| let conversationTurns = 0; | ||
| api.on("ConversationText", (data) => { | ||
| if (data.role === "user") { | ||
| conversationTurns++; | ||
| api.emit("agent_turn_started", { turn: conversationTurns, type: "user" }); | ||
| } | ||
| }); | ||
|
|
||
| // Store turn count | ||
| session._agentTurnCount = () => conversationTurns; | ||
| }, | ||
| }), | ||
| options: { trackConversations: true }, | ||
| }, | ||
| ], | ||
| }; |
There was a problem hiding this comment.
Use agentConfig to resolve no-unused-vars and demonstrate intended usage
agentConfig is declared but unused, triggering ESLint and CI failures. Calling Supervisor.attach demonstrates the intended API and resolves the lint error.
Apply this diff immediately after the agentConfig block:
const agentConfig = {
clientType: "agent",
version: "v2",
builtinMiddlewares: [
{
event: "ConversationText",
before: (payload) => {
console.log(`🔧 Agent middleware: Processing ${payload.role} message`);
},
},
],
builtinPlugins: [
{
plugin: (options = {}) => ({
attach: (session, api) => {
console.log(`🔌 Agent plugin attached with options:`, options);
// Custom agent turn tracking
let conversationTurns = 0;
api.on("ConversationText", (data) => {
if (data.role === "user") {
conversationTurns++;
api.emit("agent_turn_started", { turn: conversationTurns, type: "user" });
}
});
// Store turn count
session._agentTurnCount = () => conversationTurns;
},
}),
options: { trackConversations: true },
},
],
};
+
+ // Register custom supervision for Agent v2 (demo)
+ Supervisor.attach(agentConfig);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const agentConfig = { | |
| clientType: "agent", | |
| version: "v2", | |
| builtinMiddlewares: [ | |
| { | |
| event: "ConversationText", | |
| before: (payload) => { | |
| console.log(`🔧 Agent middleware: Processing ${payload.role} message`); | |
| }, | |
| }, | |
| ], | |
| builtinPlugins: [ | |
| { | |
| plugin: (options = {}) => ({ | |
| attach: (session, api) => { | |
| console.log(`🔌 Agent plugin attached with options:`, options); | |
| // Custom agent turn tracking | |
| let conversationTurns = 0; | |
| api.on("ConversationText", (data) => { | |
| if (data.role === "user") { | |
| conversationTurns++; | |
| api.emit("agent_turn_started", { turn: conversationTurns, type: "user" }); | |
| } | |
| }); | |
| // Store turn count | |
| session._agentTurnCount = () => conversationTurns; | |
| }, | |
| }), | |
| options: { trackConversations: true }, | |
| }, | |
| ], | |
| }; | |
| const agentConfig = { | |
| clientType: "agent", | |
| version: "v2", | |
| builtinMiddlewares: [ | |
| { | |
| event: "ConversationText", | |
| before: (payload) => { | |
| console.log(`🔧 Agent middleware: Processing ${payload.role} message`); | |
| }, | |
| }, | |
| ], | |
| builtinPlugins: [ | |
| { | |
| plugin: (options = {}) => ({ | |
| attach: (session, api) => { | |
| console.log(`🔌 Agent plugin attached with options:`, options); | |
| // Custom agent turn tracking | |
| let conversationTurns = 0; | |
| api.on("ConversationText", (data) => { | |
| if (data.role === "user") { | |
| conversationTurns++; | |
| api.emit("agent_turn_started", { turn: conversationTurns, type: "user" }); | |
| } | |
| }); | |
| // Store turn count | |
| session._agentTurnCount = () => conversationTurns; | |
| }, | |
| }), | |
| options: { trackConversations: true }, | |
| }, | |
| ], | |
| }; | |
| // Register custom supervision for Agent v2 (demo) | |
| Supervisor.attach(agentConfig); |
🧰 Tools
🪛 ESLint
[error] 25-25: 'agentConfig' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: Test / OS ubuntu-latest / Node 22
[failure] 25-25:
'agentConfig' is assigned a value but never used
[failure] 25-25:
'agentConfig' is assigned a value but never used
🪛 GitHub Check: Test / OS ubuntu-latest / Node 18
[failure] 25-25:
'agentConfig' is assigned a value but never used
[failure] 25-25:
'agentConfig' is assigned a value but never used
🤖 Prompt for AI Agents
In examples/central-supervisor-demo/index.js around lines 25 to 58, agentConfig
is declared but never used which triggers ESLint no-unused-vars and CI failures;
after the agentConfig block call the Supervisor.attach (or appropriate
Supervisor API) with agentConfig to demonstrate intended usage and register the
agent — e.g., pass agentConfig to Supervisor.attach and handle the returned
session or promise to show the plugin/middleware being attached — ensuring the
call is placed immediately after the agentConfig declaration so the variable is
used and the example runs.
| const speakConfig = { | ||
| clientType: "speak", | ||
| version: "v2", | ||
| builtinMiddlewares: [ | ||
| { | ||
| event: "Audio", | ||
| before: (payload) => { | ||
| console.log(`🔧 Speak middleware: Processing audio chunk of ${payload.length} bytes`); | ||
| }, | ||
| }, | ||
| ], | ||
| builtinPlugins: [ | ||
| { | ||
| plugin: (options = {}) => ({ | ||
| attach: (session, api) => { | ||
| console.log(`🔌 Speak plugin attached for TTS tracking`); | ||
|
|
||
| // Track TTS synthesis | ||
| let synthesisCount = 0; | ||
| api.on("Audio", () => { | ||
| synthesisCount++; | ||
| }); | ||
|
|
||
| session._synthesisCount = () => synthesisCount; | ||
| }, | ||
| }), | ||
| options: { trackSynthesis: true }, | ||
| }, | ||
| ], | ||
| }; |
There was a problem hiding this comment.
Use speakConfig to resolve no-unused-vars and demonstrate intended usage
speakConfig is also declared but unused. Attach it to the Supervisor to both showcase usage and fix lint/CI.
Apply this diff immediately after the speakConfig block:
const speakConfig = {
clientType: "speak",
version: "v2",
builtinMiddlewares: [
{
event: "Audio",
before: (payload) => {
console.log(`🔧 Speak middleware: Processing audio chunk of ${payload.length} bytes`);
},
},
],
builtinPlugins: [
{
- plugin: (options = {}) => ({
+ plugin: (_options = {}) => ({
attach: (session, api) => {
console.log(`🔌 Speak plugin attached for TTS tracking`);
// Track TTS synthesis
let synthesisCount = 0;
api.on("Audio", () => {
synthesisCount++;
});
session._synthesisCount = () => synthesisCount;
},
}),
options: { trackSynthesis: true },
},
],
};
+
+ // Register custom supervision for Speak v2 (demo)
+ Supervisor.attach(speakConfig);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const speakConfig = { | |
| clientType: "speak", | |
| version: "v2", | |
| builtinMiddlewares: [ | |
| { | |
| event: "Audio", | |
| before: (payload) => { | |
| console.log(`🔧 Speak middleware: Processing audio chunk of ${payload.length} bytes`); | |
| }, | |
| }, | |
| ], | |
| builtinPlugins: [ | |
| { | |
| plugin: (options = {}) => ({ | |
| attach: (session, api) => { | |
| console.log(`🔌 Speak plugin attached for TTS tracking`); | |
| // Track TTS synthesis | |
| let synthesisCount = 0; | |
| api.on("Audio", () => { | |
| synthesisCount++; | |
| }); | |
| session._synthesisCount = () => synthesisCount; | |
| }, | |
| }), | |
| options: { trackSynthesis: true }, | |
| }, | |
| ], | |
| }; | |
| const speakConfig = { | |
| clientType: "speak", | |
| version: "v2", | |
| builtinMiddlewares: [ | |
| { | |
| event: "Audio", | |
| before: (payload) => { | |
| console.log(`🔧 Speak middleware: Processing audio chunk of ${payload.length} bytes`); | |
| }, | |
| }, | |
| ], | |
| builtinPlugins: [ | |
| { | |
| plugin: (_options = {}) => ({ | |
| attach: (session, api) => { | |
| console.log(`🔌 Speak plugin attached for TTS tracking`); | |
| // Track TTS synthesis | |
| let synthesisCount = 0; | |
| api.on("Audio", () => { | |
| synthesisCount++; | |
| }); | |
| session._synthesisCount = () => synthesisCount; | |
| }, | |
| }), | |
| options: { trackSynthesis: true }, | |
| }, | |
| ], | |
| }; | |
| // Register custom supervision for Speak v2 (demo) | |
| Supervisor.attach(speakConfig); |
🧰 Tools
🪛 ESLint
[error] 63-63: 'speakConfig' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 76-76: 'options' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: Test / OS ubuntu-latest / Node 22
[failure] 76-76:
'options' is assigned a value but never used
[failure] 76-76:
'options' is assigned a value but never used
[failure] 63-63:
'speakConfig' is assigned a value but never used
[failure] 63-63:
'speakConfig' is assigned a value but never used
🪛 GitHub Check: Test / OS ubuntu-latest / Node 18
[failure] 76-76:
'options' is assigned a value but never used
[failure] 76-76:
'options' is assigned a value but never used
[failure] 63-63:
'speakConfig' is assigned a value but never used
[failure] 63-63:
'speakConfig' is assigned a value but never used
🤖 Prompt for AI Agents
In examples/central-supervisor-demo/index.js around lines 63 to 92, speakConfig
is defined but unused which triggers no-unused-vars; immediately after the
speakConfig block, pass speakConfig into the Supervisor (or the Supervisor
factory call) so it is consumed — e.g., when creating or configuring the
Supervisor instance include speakConfig in the constructor/options or call
supervisor.attachClient(speakConfig) depending on existing API; ensure you
import or reference the Supervisor variable in scope and remove any unused
placeholder so the config is exercised, and run lint to verify the
no-unused-vars warning is resolved.
| Supervisor.use("listen", "v2", { | ||
| event: "Results", | ||
| before: (payload) => { | ||
| console.log("🔧 Global Listen v2 middleware: Processing transcript"); | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Rename unused callback parameter to pass lint rule requiring /^_/ for unused args
The payload parameter in this global middleware isn’t used. Renaming to _payload satisfies the lint rule that allows unused args starting with _.
Apply this diff:
Supervisor.use("listen", "v2", {
event: "Results",
- before: (payload) => {
+ before: (_payload) => {
console.log("🔧 Global Listen v2 middleware: Processing transcript");
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Supervisor.use("listen", "v2", { | |
| event: "Results", | |
| before: (payload) => { | |
| console.log("🔧 Global Listen v2 middleware: Processing transcript"); | |
| }, | |
| }); | |
| Supervisor.use("listen", "v2", { | |
| event: "Results", | |
| before: (_payload) => { | |
| console.log("🔧 Global Listen v2 middleware: Processing transcript"); | |
| }, | |
| }); |
🧰 Tools
🪛 ESLint
[error] 100-100: 'payload' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: Test / OS ubuntu-latest / Node 22
[failure] 100-100:
'payload' is defined but never used
[failure] 100-100:
'payload' is defined but never used. Allowed unused args must match /^_/u
🪛 GitHub Check: Test / OS ubuntu-latest / Node 18
[failure] 100-100:
'payload' is defined but never used
[failure] 100-100:
'payload' is defined but never used. Allowed unused args must match /^_/u
🤖 Prompt for AI Agents
In examples/central-supervisor-demo/index.js around lines 98 to 104, the global
middleware function defines an unused parameter named "payload"; rename it to
"_payload" to satisfy the lint rule that unused args must start with an
underscore. Update the before callback signature from (payload) => { ... } to
(_payload) => { ... } and ensure no other references to "payload" are left in
that function body.
| const { ListenClient } = require("../../dist/main/index"); | ||
| ListenClient.use({ | ||
| event: "Results", | ||
| before: (payload) => { | ||
| // Add processing timestamp | ||
| payload._processed_at = Date.now(); | ||
| console.log("🔧 Global middleware: Processing transcript..."); | ||
| }, | ||
| after: (payload) => { | ||
| // Log processing time | ||
| const processingTime = Date.now() - payload._processed_at; | ||
| console.log(`🔧 Global middleware: Processed in ${processingTime}ms`); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Fix: Replace ListenClient.use with Supervisor.use and register a real event
ListenClient.use is not part of the exported API; use Supervisor.use("listen","v2", …). Also, “Results” may not map to a real event in the enum.
Apply this diff:
- // Register global middleware that will work with any future events
- const { ListenClient } = require("../../dist/main/index");
- ListenClient.use({
- event: "Results",
+ // Register global middleware that will work with any future events
+ const { Supervisor } = require("../../dist/main/index");
+ Supervisor.use("listen", "v2", {
+ event: LiveTranscriptionEvents.Transcript,
before: (payload) => {
// Add processing timestamp
payload._processed_at = Date.now();
console.log("🔧 Global middleware: Processing transcript...");
},
after: (payload) => {
// Log processing time
const processingTime = Date.now() - payload._processed_at;
console.log(`🔧 Global middleware: Processed in ${processingTime}ms`);
},
});Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[error] 7-7: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
🤖 Prompt for AI Agents
In examples/node-listen-v2-flux-ready/index.js around lines 7 to 20, replace the
non-existent ListenClient.use call with Supervisor.use for the listen v2
integration and register a real event: import Supervisor from the library
instead of ListenClient, call Supervisor.use("listen","v2", { event:
"<VALID_EVENT_NAME>", before: ..., after: ... }), and set event to one of the
actual exported listen event names (check the package's exported enum/constants
and replace "Results" with that valid value).
| const { createClient, LiveTranscriptionEvents, ListenV2 } = require("@deepgram/sdk"); | ||
|
|
||
| // Global middleware (applies to all v2 sessions) | ||
| ListenV2.use({ | ||
| event: "Results", | ||
| before: (payload) => { | ||
| console.log("Processing transcript globally..."); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Fix API reference: replace non-existent ListenV2/use with Supervisor.use('listen','v2', …)
The README shows ListenV2.use, but the SDK surface in this PR exposes Supervisor.use(clientType, version, middleware). Using ListenV2 will mislead consumers and examples won’t run.
Apply this diff to align with the actual API:
-const { createClient, LiveTranscriptionEvents, ListenV2 } = require("@deepgram/sdk");
+const { createClient, LiveTranscriptionEvents, Supervisor } = require("@deepgram/sdk");
@@
-// Global middleware (applies to all v2 sessions)
-ListenV2.use({
+// Global middleware (applies to all v2 sessions)
+Supervisor.use("listen", "v2", {
event: "Results",
before: (payload) => {
console.log("Processing transcript globally...");
},
-});
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { createClient, LiveTranscriptionEvents, ListenV2 } = require("@deepgram/sdk"); | |
| // Global middleware (applies to all v2 sessions) | |
| ListenV2.use({ | |
| event: "Results", | |
| before: (payload) => { | |
| console.log("Processing transcript globally..."); | |
| }, | |
| }); | |
| const { createClient, LiveTranscriptionEvents, Supervisor } = require("@deepgram/sdk"); | |
| // Global middleware (applies to all v2 sessions) | |
| Supervisor.use("listen", "v2", { | |
| event: "Results", | |
| before: (payload) => { | |
| console.log("Processing transcript globally..."); | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In examples/node-listen-v2-flux-ready/README.md around lines 35 to 43, the
example uses ListenV2.use which does not exist in the SDK; replace that usage
with Supervisor.use('listen', 'v2', ...) and update the imports/require list to
import Supervisor (and remove ListenV2 if present). Ensure the middleware object
stays the same (event: "Results", before: (payload) => { ... }) and call
Supervisor.use('listen', 'v2', { event: "Results", before: (payload) => {
console.log("Processing transcript globally..."); } }) so the example matches
the actual API.
| const { createClient, LiveTranscriptionEvents } = require("../../dist/main/index"); | ||
| require("dotenv").config(); |
There was a problem hiding this comment.
Fix ESLint: no-require-imports rule is breaking CI
The examples are using CommonJS require, but the examples ESLint config forbids it. Easiest unblock is to disable the rule for this file (or convert to ESM everywhere, which is heavier). This patch silences the rule locally.
Apply this diff:
+/* eslint-disable @typescript-eslint/no-require-imports */
const { createClient, LiveTranscriptionEvents } = require("../../dist/main/index");
require("dotenv").config();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { createClient, LiveTranscriptionEvents } = require("../../dist/main/index"); | |
| require("dotenv").config(); | |
| /* eslint-disable @typescript-eslint/no-require-imports */ | |
| const { createClient, LiveTranscriptionEvents } = require("../../dist/main/index"); | |
| require("dotenv").config(); |
🧰 Tools
🪛 ESLint
[error] 1-1: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 2-2: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
🤖 Prompt for AI Agents
In examples/version-patterns-demo/index.js around lines 1 to 2, ESLint's
no-require-imports rule is failing because the file uses CommonJS require; to
fix, add a file-level ESLint disable for that rule at the top of this file (e.g.
insert a single-line or block comment to disable no-require-imports) so the
existing require calls are allowed, keeping the rest of the file unchanged.
| export { Supervisor } from "./supervisor/Supervisor"; | ||
| export type { Middleware, MiddlewareContext } from "./middleware/types"; | ||
| export type { SupervisorConfig } from "./supervisor/Supervisor"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Export enhanced ListenClient and add a ListenV2 convenience wrapper to match docs/DX
To make ListenClient.use feasible (if added) and to provide a simple global entry for v2 middleware, export the SDK ListenClient and an alias object.
export { Supervisor } from "./supervisor/Supervisor";
export type { Middleware, MiddlewareContext } from "./middleware/types";
export type { SupervisorConfig } from "./supervisor/Supervisor";
+// Ensure the enhanced ListenClient (with supervision) is exported from the SDK, not the core.
+export { ListenClient } from "./ListenClient";
+// Convenience: global registration target for Listen v2 middlewares
+export const ListenV2 = {
+ use(middleware: any) {
+ // proxy to Supervisor with fixed clientType/version
+ Supervisor.use("listen", "v2", middleware);
+ },
+};If you’d rather keep examples using Supervisor.use directly, skip exporting ListenV2 and drop ListenClient.use.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/sdk/index.ts around lines 61 to 64, export the SDK's ListenClient (the
concrete client class/type) in addition to the existing Supervisor exports, and
add a convenience alias/object named ListenV2 that references/re-exports
ListenClient so docs/examples can import a single v2 entry (e.g., export
ListenClient from its module and export a ListenV2 object that exposes
ListenClient and any related v2 helpers/types). Ensure any related types used by
ListenClient are also exported if necessary.
| private static registerMiddlewares(registry: MiddlewareRegistry, config: SupervisorConfig): void { | ||
| if (config.middlewares) { | ||
| config.middlewares.forEach((middleware) => { | ||
| MiddlewareRegistry.use(middleware); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: middlewares registered per (clientType-version) are stored globally
registerMiddlewares calls MiddlewareRegistry.use (static), which pushes into a static global array inside the registry. This defeats the per clientType-version isolation and can leak middlewares across namespaces/versions.
Apply this minimal fix first:
private static registerMiddlewares(registry: MiddlewareRegistry, config: SupervisorConfig): void {
if (config.middlewares) {
config.middlewares.forEach((middleware) => {
- MiddlewareRegistry.use(middleware);
+ registry.use(middleware);
});
}
}Additionally, adjust MiddlewareRegistry to keep middlewares per registry instance (see patch below).
- /** Global middlewares that apply to all sessions */
- private static globalMiddlewares: Middleware[] = [];
+ /** Middlewares that apply to sessions using this registry instance */
+ private globalMiddlewares: Middleware[] = [];
@@
- static use(middleware: Middleware): void {
- this.globalMiddlewares.push(middleware);
- }
+ // Remove the static registration; keep per-instance registration only.
@@
use(middleware: Middleware): void {
- // For now, add to global middlewares, but this could be made registry-specific
- MiddlewareRegistry.globalMiddlewares.push(middleware);
+ this.globalMiddlewares.push(middleware);
}
@@
getMiddlewares(session: AbstractLiveClient, event: string): Middleware[] {
- const global = MiddlewareRegistry.globalMiddlewares.filter((mw) => mw.event === event);
+ const global = this.globalMiddlewares.filter(
+ (mw) => mw.event === event || mw.event === "*"
+ );
const instance =
- this.instanceMiddlewares.get(session)?.filter((mw) => mw.event === event) || [];
+ this.instanceMiddlewares
+ .get(session)
+ ?.filter((mw) => mw.event === event || mw.event === "*") || [];
return [...global, ...instance];
}Note: Remove all remaining call sites to the static MiddlewareRegistry.use; after the Supervisor.ts change above, there should be none.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static registerMiddlewares(registry: MiddlewareRegistry, config: SupervisorConfig): void { | |
| if (config.middlewares) { | |
| config.middlewares.forEach((middleware) => { | |
| MiddlewareRegistry.use(middleware); | |
| }); | |
| } | |
| private static registerMiddlewares(registry: MiddlewareRegistry, config: SupervisorConfig): void { | |
| if (config.middlewares) { | |
| config.middlewares.forEach((middleware) => { | |
| registry.use(middleware); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In src/sdk/supervisor/Supervisor.ts around lines 80 to 85, the current
registerMiddlewares uses the static MiddlewareRegistry.use which appends to a
global static array and breaks per-clientType/version isolation; change
registerMiddlewares to call the registry instance (e.g.,
registry.use(middleware)) instead of the static method, and ensure all call
sites no longer call MiddlewareRegistry.use. Also update MiddlewareRegistry so
it stores middlewares on the instance (non-static array), expose an instance
method use(middleware) that pushes into that instance array, remove or deprecate
the static use/remove methods, and run a project-wide search to replace
remaining static MiddlewareRegistry.use calls with instance calls so middlewares
are kept per registry.
| (session as any).emit = (event: string, ...args: any[]) => { | ||
| const payload = args[0]; | ||
| const ctx: MiddlewareContext = { | ||
| session, | ||
| turnCount: (session as any)._turnCount?.() || 0, | ||
| connectionAttempt: (session as any)._connectionAttempt || 0, | ||
| metadata: (session as any)._lastMetadata, | ||
| resumeContext: (session as any)._resumeContext, | ||
| }; | ||
|
|
||
| // Execute middlewares asynchronously but don't block the event emission | ||
| this.executeMiddlewares(session, event, payload, ctx, originalEmit, registry); | ||
|
|
||
| // Return the original emission result immediately | ||
| return originalEmit(event, ...args); | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Non-blocking “before” hook runs after event is emitted—contradicts expectations/examples
wrapEventEmission fires originalEmit immediately and runs executeMiddlewares asynchronously. This means “before” hooks do not run before listeners, so payload mutations aren’t visible in listeners. The example relies on _processed_at being set before Transcript handlers run.
Two paths forward:
- Semantics change (recommended): Await “before” then emit, and schedule “after” asynchronously. This preserves EventEmitter behavior for listeners while keeping “after” non-blocking. If changing the return type of emit is unacceptable, constrain “before” to be synchronous-only and run it inline (documented).
- Documentation change: Explicitly state “before” hooks are non-blocking and may run after listener execution. Update examples to avoid reading mutations in listeners.
I can prepare a PR for either approach. Preference?
🤖 Prompt for AI Agents
In src/sdk/supervisor/Supervisor.ts around lines 117–133, the current
wrapEventEmission calls originalEmit immediately and runs executeMiddlewares
asynchronously, causing "before" hooks to run after listeners; change behavior
so "before" middlewares run and complete before emitting, then emit and return
originalEmit's result, and finally run "after" middlewares asynchronously.
Concretely: await execution of the middleware "before" phase using
executeMiddlewares (or split executeMiddlewares into before/after phases), call
and return originalEmit(event, ...args) once before-middlewares finish, and
schedule the "after" phase to run without blocking the return value; if awaiting
before hooks is impossible due to emit signature constraints, instead run before
hooks synchronously inline and document that requirement.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/sdk/ListenClient.ts (1)
35-53: Avoid unconditional console logging and potential PII leakage in middleware hooks.Logging payload and ctx on every SpeechStarted event can leak sensitive data (session metadata, resume context, request IDs) in user apps and browser consoles.
Apply this diff to gate logs to test/dev and avoid logging ctx:
before: (payload: any, ctx: MiddlewareContext) => { - console.log("before:SpeechStarted", payload, ctx); + if (typeof process !== "undefined" && process?.env?.NODE_ENV === "test") { + console.log("before:SpeechStarted", payload); + } }, after: (payload: any, ctx: MiddlewareContext) => { - console.log("after:SpeechStarted", payload, ctx); + if (typeof process !== "undefined" && process?.env?.NODE_ENV === "test") { + console.log("after:SpeechStarted", payload); + } },If you intend to keep structured observability, prefer a pluggable logger via SDK options rather than console.*. I can propose a minimal logger interface if desired.
🧹 Nitpick comments (4)
src/sdk/ListenClient.ts (4)
18-30: Delegate to super.live and mark as override to avoid duplication and drift.Constructing CoreListenLiveClient directly duplicates core logic and risks divergence if the core's constructor signature or setup changes.
Apply this diff:
- public live( + public override live( transcriptionOptions: LiveSchema = {}, endpoint: string = ":version/listen" ): CoreListenLiveClient { - // Create the core live client - const coreClient = new CoreListenLiveClient(this.options, transcriptionOptions, endpoint); + // Delegate to core implementation to avoid duplication + const coreClient = super.live(transcriptionOptions, endpoint) as CoreListenLiveClient; - if (this.shouldSupervise("v2")) { + if (this.shouldSupervise("v2", endpoint)) { this.attachListenV2Supervision(coreClient); } return coreClient; }Also consider adding unit coverage for:
- supervision attached for v2 endpoints
- supervision not attached for non-v2 endpoints
I can scaffold a unit test that spies on Supervisor.attach and asserts call patterns if helpful.
25-27: Make supervision decision endpoint-aware, avoid magic strings, and clarify param naming.JSDoc says “based on endpoint and version” but implementation only checks version. Also, parameter name “version” shadows the class property semantically; use a clearer name and consider endpoint signal.
Apply this diff:
- if (this.shouldSupervise("v2")) { + if (this.shouldSupervise("v2", endpoint)) { this.attachListenV2Supervision(coreClient); }- private shouldSupervise(version: string): boolean { - const isV2 = this.version === version; - return isV2; - } + private shouldSupervise(targetVersion: string, endpoint?: string): boolean { + const versionMatches = this.version === targetVersion; + const endpointMatches = endpoint ? endpoint.includes("v2") : false; + return versionMatches || endpointMatches; + }Additionally, consider centralizing API versions in a constant/enum to avoid scattered "v2" magic strings.
Also applies to: 58-61
37-38: Use this.namespace instead of hard-coded "listen".Leverage inherited namespace to reduce duplication and ease future renames.
Apply this diff:
- clientType: "listen", + clientType: this.namespace,
18-21: Return type doesn’t reflect the runtime “use()” augmentation added by Supervisor.Supervisor.attach decorates the session with a use(middleware) method, but the signature still advertises CoreListenLiveClient. This hides a public capability from TypeScript consumers.
Two options:
- Minimal change (intersection type):
- ): CoreListenLiveClient { + ): CoreListenLiveClient & { use: (middleware: unknown) => CoreListenLiveClient } {
- Better DX: define and export an EnhancedListenLiveClient type that augments the core client, then use it as the return type. For example:
// at top-level of this file (or a shared types module) export type EnhancedListenLiveClient = CoreListenLiveClient & { use: (middleware: unknown) => EnhancedListenLiveClient; };Then change the method signature to return EnhancedListenLiveClient.
Also applies to: 29-30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/sdk/ListenClient.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/sdk/ListenClient.ts (5)
src/core/packages/ListenClient.ts (1)
ListenClient(13-35)src/core/lib/types/TranscriptionSchema.ts (1)
LiveSchema(246-246)src/sdk/supervisor/Supervisor.ts (2)
SupervisorConfig(8-40)Supervisor(45-190)src/sdk/middleware/types.ts (1)
MiddlewareContext(6-21)src/core/lib/version.ts (1)
version(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Browser Builds
🔇 Additional comments (2)
src/sdk/ListenClient.ts (2)
41-49: Confirm “SpeechStarted” event exists in core clientOur search didn’t locate any
emit("SpeechStarted")oron("SpeechStarted")calls in the codebase. Please manually verify that the core client actually emits this event with exactly that casing—otherwise these middleware hooks will never run. If a specific TypeScript type exists for the event payload, replace the currentanywith it for stronger typing.• File: src/sdk/ListenClient.ts, lines 41–49
1-6: LiveSchema import is valid via barrel exportConfirmed that
src/core/lib/types/index.tsexists and re-exportsLiveSchema(fromTranscriptionSchema.ts), so importing withfrom "../core/lib/types"will resolve correctly. No changes needed.
| ], | ||
| }; | ||
|
|
||
| Supervisor.attach(session, config); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check that Supervisor.attach actually registers the provided middlewares.
Based on the provided Supervisor snippet, registerMiddlewares calls MiddlewareRegistry.use(...) statically rather than using the registry instance, which could no-op or apply to a different global. If so, the config.middlewares defined here may not run.
Run:
If it’s indeed using a static, consider changing Supervisor to call registry.use(mw) so the config here reliably applies.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the registerMiddlewares implementation
rg -nP -C5 'registerMiddlewares\(registry:\s*MiddlewareRegistry' src/sdk/supervisor
# Confirm that instance registry is used to register provided middlewares
rg -nP -C2 'class\s+MiddlewareRegistry|static\s+use\(|use\(' src/sdk/middlewareLength of output: 2060
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the implementation of Supervisor.attach to verify middleware registration
rg -nP -C10 'static attach' src/sdk/supervisor/Supervisor.tsLength of output: 961
Ensure per-instance middleware registration
Supervisor.registerMiddlewares is currently calling the static MiddlewareRegistry.use(...), ignoring the passed-in registry instance and always appending to the global list. As a result, config.middlewares won’t be scoped to the specific client type/version.
• File – src/sdk/supervisor/Supervisor.ts
• Lines 82–83 (inside registerMiddlewares)
Change from:
config.middlewares.forEach((middleware) => {
MiddlewareRegistry.use(middleware);
});To:
config.middlewares.forEach((middleware) => {
registry.use(middleware);
});This ensures custom middlewares are registered on the correct MiddlewareRegistry instance.
🤖 Prompt for AI Agents
In src/sdk/ListenClient.ts around line 52 and src/sdk/supervisor/Supervisor.ts
around lines 82–83, registerMiddlewares is calling the static
MiddlewareRegistry.use(...) instead of using the passed-in registry instance,
causing global middleware leakage; update the loop to call
registry.use(middleware) for each config.middleware (and ensure the registry
parameter is properly passed into registerMiddlewares and not shadowed) so
middlewares are registered on the correct MiddlewareRegistry instance for this
client/version.
1 participant
Proposed changes
Types of changes
What types of changes does your code introduce to the community JavaScript SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
Summary by CodeRabbit
New Features
Documentation
Tests
Chores