Upgrade Python/Node bindings, enable async analyzer - #38
Merged
Merged
Conversation
The Python API is unchanged: the .pyi files for 3.1.0 and 3.2.0 are byte-identical, so no plugin code changes. What moves is the native core underneath, which get_sdk_version() reports as 0.23.0 for aic-sdk 3.1.0 and 0.24.0 for 3.2.0. Raise the floor to 3.2 rather than only relocking. The old >=3.1.0,<4 range already admitted 3.2.0, but it also still admitted core 0.23, leaving the resolved core up to chance. Wheels cover cp310 through cp314 on every supported platform, and the compatible model version stays at 7, so provisioned models keep working. This puts Python on core 0.24 ahead of Node, which follows in the next commits; until then the two packages run different cores. Python gains no dispose() in 3.2 and keeps relying on binding finalization. Verified with unit tests, the integration suite against the real SDK, ruff, and mypy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aic-sdk 0.24 reshapes the Node API. Adapt to it without changing plugin behavior; the async analysis it unlocks follows separately. The separate Collector and Analyzer natives created by analyzerPair() are merged into a single Analyzer class carrying both initialize()/buffer() and analyze(). Our public Collector and Analyzer now hold the same native instance, Collector calling only the buffering half. Its close path also disposes the native handle rather than leaving it to garbage collection. The package ships TypeScript declarations now, so drop the hand-written aic-sdk.d.ts and reduce sdk.ts to a re-export boundary. That ambient module declaration was shadowing the package's own types, which is why tsc kept passing against an API that no longer exists. ProcessorParameter and VadParameter stay hand-mirrored because they are const enums with no runtime form, and the plugin re-exports ProcessorParameter publicly. Two further renames ride along: VadContext.rawVadProbability() is now getRawVadProbability(), and Model.download() returns a promise, so the README and the tests that provision models await it. Python is unaffected: aic-sdk 3.2 still exposes analyzer_pair(), and the mirrored plugin objects keep the same shape in both runtimes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aic-sdk 0.24 adds Analyzer.analyzeAsync(), which runs inference on a libuv worker thread. Use it instead of the synchronous analyze(), so periodic analysis no longer freezes the agent's event loop. Measured against the real SDK with a tyto-1.1-l model and a 5 ms timer: the synchronous call blocked for its full 120 ms of inference and the timer fired zero times, while analyzeAsync() let it fire 22 times with 0.4 ms worst-case lag. Buffering does not take the analyzer lock, so the collector keeps feeding audio for the whole duration. A tick arriving while an analysis is still in flight is skipped rather than queued, reported through a rate-limited warning carrying the skip count. This matches how the Processor and VAD report falling behind. close() now returns a promise and releases the native analyzer only once any in-flight analysis has settled, because terminateSession() and dispose() wait for the analyzer lock. It stays safe to call without awaiting, which is how RoomIO closes the collector, and repeated calls return the same promise. This mirrors the aclose() shutdown path Python already has. Verified with the integration suite against the real SDK. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The analyzer upgrade already released its native instance explicitly; do the same for the other two components rather than leaving half the plugin relying on garbage collection. Both close paths were already shaped for this: each clears its native references before tearing down, and every process() guards on them, so nothing can reach a disposed instance. Confirmed against the real SDK that terminateSession() followed by dispose() succeeds, that repeated disposal is a no-op, and that use after disposal throws. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
livekit-plugins/node/src/analyzer.ts
Line 320 in 5c9e561
Capture stream metadata before starting async analysis
When stream information changes while analyzeAsync() is in flight, onStreamInfoUpdated() replaces currentStreamInfo before the promise continuation runs. The completed result still describes audio collected for the previous stream, but this lookup labels the event with the new room participant/publication, corrupting per-participant analysis; snapshot the stream metadata when scheduling the analysis and use that snapshot for the result and diagnostics.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The analyzer read the collector's current stream info after awaiting inference. A stream change lands on the collector meanwhile, replacing that info and resetting the buffer, so the completed result was emitted and logged with the new participant and publication while describing audio collected for the previous one. Snapshot the stream info when scheduling an analysis and carry it through the event and the diagnostics. A stream change resets the collector, so the audio being analyzed always belongs to the stream current at that point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment claimed ProcessorParameter and VadParameter have no runtime form of their own. napi-rs does export both from aic-sdk's index.js, with members matching the declarations; only TypeScript's const enum treatment is compile-time-only. State that instead, and keep the reason for the hand-written mirrors: the plugin owns the objects it re-exports rather than leaning on declarations a bundler may erase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Codex's first review was legit, and it exposed that the same problem was already present in the Python plugin. Both are fixed in 4ffa756. |
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This updates the underlying Python bindings to version 3.2 and the Node bindings to 0.24.
This has no API changes for Python, but for Node the model download function became
async. Instead of ...... it is now used like this:
The analyzer now runs asynchronously on on Node's libuv thread pool, see also https://github.com/ai-coustics/aic-sdk-node/releases/tag/0.24.0.