Skip to content

Rewrite SDK with napi-rs - #22

Merged
steckes merged 21 commits into
mainfrom
next
Sep 7, 2026
Merged

steckes merged 21 commits into
mainfrom
next

Conversation

@steckes

@steckes steckes commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Rewritten on napi-rs, on top of the public aic-sdk Rust crate.

The previous release was a raw native addon exporting low-level primitives, wrapped in a
hand-written JavaScript ergonomics layer and documented only with JSDoc. That layer is gone:
the binding and its type declarations are now generated from annotated Rust.

Added

  • TypeScript declarations. index.d.ts ships with the package, with doc comments on
    every class, method and enum member. Previous releases published no types.
  • Analyzer.analyzeAsync, running the analysis model on a worker thread. The SDK's
    analysis models are too expensive for an audio thread, and Node cannot move the analyzer
    into a worker the way the Rust SDK's collector/analyzer split allows, so this is how that
    capability is reached here. There is no AnalyzerAsync class: only this one call moves
    off-thread, and buffer stays synchronous and lock-free so audio can keep arriving while
    an analysis is in flight.
  • ProcessorAsync and VadAsync, mirroring the Rust SDK. Each call returns a promise
    and runs on Node's libuv thread pool, keeping enhancement and detection off the event
    loop. process copies its input and resolves to the samples rather than writing in place,
    so the caller's array stays valid while the promise is pending. Parallelism is across
    instances: give each stream its own, and raise UV_THREADPOOL_SIZE to run more than four
    at once.
  • Model.download is asynchronous and resolves to the model path, so a cold download no
    longer blocks the event loop.

Changed

The API was modernized:

0.23 0.24
Model.download(...) (blocking) await Model.download(...)
analyzerPair(model, key) new Analyzer(model, key)
collector.buffer(...) analyzer.buffer(...)
OtelConfig.enabled() { enable: true }
  • analyzerPair() and the separate Collector are replaced by a single Analyzer class
    with buffer() and analyzeBuffered(). The SDK separates collection from analysis so the
    halves can live on different threads; that does not apply in Node, where an instance cannot
    cross into another worker.
  • OtelConfig is a plain object ({ enable, sessionId?, exportIntervalMs? }) rather than a
    class with static factories, still passed as the optional third constructor argument.
  • ProcessorParameter and VadParameter are real enums with stable numeric values
    (Bypass = 0, EnhancementLevel = 1; SpeechHoldDuration = 0, Sensitivity = 1,
    MinimumSpeechDuration = 2).
  • Minimum supported Node version is 18.

Removed

  • FileAnalyzer. Its convenience windowing is not yet reimplemented on the new binding;
    window over Analyzer directly in the meantime.

Replace the previous implementation with the napi-rs based rewrite from
aic-sdk-node-new, including async APIs, examples, CI, and tests.
@steckes
steckes requested review from Fl1tzi, andresovela and mgeier and removed request for mgeier August 25, 2026 07:54
steckes and others added 8 commits August 25, 2026 10:12
The sync task called `processor.process(audio)` on the buffer the async tasks
also read. tinybench runs tasks in registration order, so by the time the async
task started, that block had been enhanced hundreds of thousands of times: the
two tasks were measuring different signals, and the comment claiming the gap is
the promise plus the copy compared unlike workloads.

The sync task now enhances a scratch buffer refilled from `audio` in an untimed
`beforeEach`, so every task starts from the same reference signal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The combined loop warned at length that the VAD must see the original audio,
then assigned the processor's output back to `block`, so every iteration after
the first detected on enhanced audio — exactly the mistake the comment forbids,
in the lines a reader is most likely to copy.

Each iteration now takes a fresh block, as a real stream would, and logs the
detection alongside the enhanced output.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The quick start is CommonJS but used top-level `await Model.download(...)`,
which is a syntax error in a .js file — the first snippet a new user meets did
not run when pasted into a script. It now wraps the body in the `async function`
it needs, and a note says the later snippets omit that wrapper on purpose.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rewrite left CHANGELOG.md holding 0.24.0 alone, dropping every entry from
0.23.0 back to 0.6.3 and undoing 3f60b49, which had just made the file
accumulate. Anyone upgrading from an older release lost the record of what
changed in between.

The restored entries are reformatted by the current Prettier config, which
changed quote and semicolon style in embedded snippets after the rewrite. No
wording changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The napi-rs group matched "@napi/cli", which is not a published package — the
CLI is "@napi-rs/cli" — so CLI bumps landed ungrouped and unlabelled. The second
rule keyed on eslint and @typescript-eslint, neither of which is a dependency
since the move to oxlint, so it could never match; it now covers oxlint,
prettier and taplo.

"config:base" and "matchPackagePatterns" are both deprecated and warn on every
run, replaced by "config:recommended" and exact "matchPackageNames".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
index.js and index.d.ts are committed but regenerated by `napi build`, and
nothing compared the two. A changed Rust signature or doc comment without a
local rebuild passed CI and shipped type declarations that did not match the
addon; typecheck:examples only caught it when an example happened to use the
changed symbol.

The Linux x64 build now diffs the regenerated files against the committed ones.
One target suffices: both files are generated from `napi.targets`, not from the
target being built.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Commit e49263f wrapped the model catalogue URL in angle brackets in the
Rust doc comment but did not rebuild, so the generated typings drifted.
The staleness check added in e5a3a1e caught it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@andresovela

Copy link
Copy Markdown
Collaborator

analyzerPair() and the separate Collector are replaced by a single Analyzer class
with buffer() and analyzeBuffered(). The SDK separates collection from analysis so the
halves can live on different threads; that does not apply in Node, where an instance cannot
cross into another worker.

I don't understand this. If that were the case, there's also no point in keeping the Context types around, since they're meant to be used across threads?

Comment thread index.d.ts
Comment thread src/analyzer.rs
Comment thread scripts/fetch-test-models.mjs
Comment thread index.js
Comment thread README.md Outdated
Comment thread src/lib.rs Outdated
@steckes

steckes commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@andresovela The only way to use background threads in napi seems to be via the async functions which execute on the uv thread pool. That happens when you call analyzeAsync. For the contexts on the other hand, it does make sense to keep them separate, because you still want to call the functions on the context while the process function is being called on the background thread. The Collector / Analyzer split does not offer any functionality in that regard.

This will help with the confusion between analyze_buffered and
analyze_async as it felt like these functions are for two different
things.
@steckes
steckes requested a review from mgeier August 28, 2026 09:12
@andresovela

Copy link
Copy Markdown
Collaborator

@steckes how would you write code that analyzes the output of a Quail model for example? You'd have to call analyze in the same thread dedicated for audio processing

@steckes

steckes commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Everything runs on the main event loop also your incoming audio. From there you run analyze_async which then runs the analyzer on the background thread without blocking your event loop. There is no dedicated audio thread or similar in node afaik where you could push the analyzer inside or where you process the audio.

for (const block of audioBlocks) {
  processor.process(block)      // enhance in place
  analyzer.buffer(block)        // lock-free, cheap
}

const result = await analyzer.analyzeAsync()  // model runs on a worker

@andresovela

Copy link
Copy Markdown
Collaborator

That example runs the analyzer in a background thread, but blocks the current thread from processing further audio until the analysis is complete. Ideally you'd analyze independently in a different task, and then the Collector/Analyzer split is the way to go.

@mgeier mgeier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending the resolution of @andresovela's comments.

@andresovela andresovela left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've come to terms with the new Analyzer API. I think we can leave it as proposed in this PR. However, I do think the docs of the new APIs need to be rewritten as we have them as we have them in other SDKs. They have diverged a lot in this PR, and several doc-comments read very AI-generated.

For example:
This PR:

/// Analyzer for analysis models such as Tyto.
///
/// Buffering and analysis are deliberately separate calls: {@link Analyzer#buffer} is cheap
/// enough for the audio path, while running the model is not. Analysis therefore comes in
/// two forms: {@link Analyzer#analyzeAsync} on a worker thread, and
/// {@link Analyzer#analyze} on the calling thread.
///
/// Only a fixed span of audio is retained, determined by the model; older audio is
/// discarded as more is buffered.
///
/// The SDK splits this into a collector and an analyzer so the two halves can live on
/// different threads. A class instance cannot cross into a Node worker, so both are exposed
/// as one object here, but the split still shows through: {@link Analyzer#buffer} drives
/// the collector on the calling thread, while {@link Analyzer#analyzeAsync} moves the
/// analyzer half onto a worker. The SDK guarantees the two are safe to use concurrently.

vs Rust SDK:

/// Creates a collector/analyzer pair for non-real-time analysis.
///
/// The collector is designed to be placed in the audio thread, buffering audio chunks for
/// later analysis.
///
/// The analyzer is designed to be run separately. Analysis models are computationally expensive
/// and cannot run in the audio thread. The analyzer has access to the audio buffered by the
/// collector, and it can access it safely across threads.
///
/// The collector retains a span of audio determined by the analysis model. As more samples
/// get collected, old audio is discarded.

Comment thread src/processor_async.rs Outdated
Comment thread Cargo.toml Outdated
steckes and others added 4 commits September 7, 2026 20:38
Document the SDK-internal error reporting from the underlying SDK and date the
0.24.0 release.

Also backfill the 0.23.1 section, which shipped without a changelog entry: the
cached artifact manifest and the 30 second timeout on manifest requests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The SDK is a complete rewrite on this branch, so the merge keeps the
'next' tree as-is and discards everything unique to main. This records
main as an ancestor so next can be merged into main without conflicts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@steckes
steckes merged commit 2a90aac into main Sep 7, 2026
14 checks passed
@steckes
steckes deleted the next branch September 7, 2026 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants