Conversation
fcca2c4 to
d32ff07
Compare
i386
changed the title
ndizazzo
left a comment
ndizazzo
left a comment
There was a problem hiding this comment.
Nothing jumps out as wrong, but I had a thought about the boundary of how these SDKs are split...
I think it might be worth thinking about splitting the client and host surfaces at the crate level before the FFI stabilizes. This would allow apps to include smaller SDKs to either:
- get inference
- be inference
The Rust types already is almost there with the separation (MeshClient vs MeshNode) but RN they're in the same crate and compile to the same artifact.
A smaller mobile SDK consumer wouldn't need llama.cpp, skippy, or the runtime control surface. As it stands, now they'd ship all of it.
Something like:
- mesh-llm-api-client: inference, discovery, catalog, identity, events. The dependency footprint stays light (QUIC, protobuf, HF download, catalog).
- mesh-llm-api-node: re-exports client + adds ServingController, model loading, device policy, runtime plumbing.
At the FFI boundary you'd get two distinct handles (ClientHandle vs NodeHandle) which maps cleanly to Swift/Kotlin consumers. no runtime "serving_enabled" boolean that returns errors when you call something that shouldn't exist on a client. Better type safety, smaller binaries, and the UniFFI interface stays clean from the start instead of needing a breaking change later.
The Node.js addon could keep the same single entry point with its existing feature flag.
| "modelRef": model.model_ref, | ||
| "modelId": model.model_id, | ||
| "instanceId": model.instance_id, | ||
| "state": format!("{:?}", model.state), |
There was a problem hiding this comment.
format!("{:?}", ...) ties the JSON API contract to Rust's Debug trait output. For ServingModelState::Unknown("x"), Debug produces Unknown("x") while UniFFI's tagged enum encoding produces { "Unknown": "x" } — an inconsistency across SDK targets.
More generally, Debug is an unstable serialization contract. These should use explicit match arms or serde Serialize instead.
| "vision": format!("{:?}", value.vision), | ||
| "audio": format!("{:?}", value.audio), | ||
| "reasoning": format!("{:?}", value.reasoning), | ||
| "toolUse": format!("{:?}", value.tool_use), |
| fn set_device_policy<'a>(&'a self, policy: NodeDevicePolicy) -> ServingFuture<'a, ()> { | ||
| Box::pin(async move { | ||
| match policy { | ||
| NodeDevicePolicy::Auto => Ok(()), | ||
| policy => Err(anyhow::anyhow!(ServingError::UnsupportedDevicePolicy { | ||
| policy, | ||
| })), | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
When an SDK consumer uses the host runtime path (not EmbeddedServingController), passing DevicePolicy::Cpu or DevicePolicy::Gpu fails with an error.
The EmbeddedServingController accepts all policies. This means node.serving().set_device_policy(...) silently fails depending on which backend is active.
Needs at minimum a doc note or a consistent behavior path.
| Failed(); | ||
| Unloading(); | ||
| Stopped(); | ||
| Unknown(string value); |
There was a problem hiding this comment.
"state": format!("{:?}", model.state)
// Produces: "Unknown(\"some state\")" in Node.js
vs in the UDL:
[Enum]
interface ServingModelState {
Unknown(string value);
};
// Produces: { "Unknown": "some state" } via UniFFI in Swift/Kotlin
A developer switching between Kotlin and Node.js gets fundamentally different representations for the same enum variant. Node.js should normalize to match: { "type": "Unknown", "value": "..." }
| } | ||
|
|
||
| impl ServingController for EmbeddedServingController { | ||
| fn load<'a>(&'a self, request: LoadModelRequest) -> ServingFuture<'a, ServedModel> { |
There was a problem hiding this comment.
The runtime path in api/mod.rs goes through reserve_runtime_capacity_for_model with a capacity ledger.
This calls SkippyModelHandle::load(options) unconditionally and relies on skippy to fail on OOM — producing native log errors rather than structured error messages.
Potentially consider at least a total_vram_bytes gate before loading.
| pub async fn model_list(&self) -> Vec<(String, String)> { | ||
| self.inner | ||
| .lock() | ||
| .await | ||
| .models | ||
| .values() | ||
| .map(|model| (model.served.model_id.clone(), model.served.model_id.clone())) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
Both tuple fields use model_id. If the same model is loaded with two different instance_ids, model_list returns duplicates.
Downstream callers like FFI::inference_list_models use this directly.
| let outstanding_refs = std::sync::Arc::strong_count(&rt); | ||
| if outstanding_refs == 1 { | ||
| let dir = rt.dir().to_path_buf(); | ||
| drop(rt); | ||
| let _ = std::fs::remove_dir_all(&dir); | ||
| } else { | ||
| tracing::warn!( | ||
| outstanding_refs, | ||
| "skipping runtime directory removal during shutdown because runtime references remain" | ||
| ); | ||
| } |
There was a problem hiding this comment.
Arc::strong_count is a snapshot, not a guarantee. Between the count check and drop(rt), another thread could clone the Arc. Consider Arc::try_unwrap instead:
if let Ok(rt) = Arc::try_unwrap(rt) {
let dir = rt.dir().to_path_buf();
drop(rt);
let _ = std::fs::remove_dir_all(&dir);
}There was a problem hiding this comment.
This file is WAY too long
| function loadNativeFile(file) { | ||
| const resolved = path.resolve(file) | ||
| const mod = { exports: {} } | ||
| process.dlopen(mod, resolved) | ||
| return mod.exports | ||
| } |
There was a problem hiding this comment.
This bypasses Node.js module caching and require interception hooks. The addon can be loaded multiple times (no singleton cache) and require.resolve() won't work for diagnostics.
Worth a comment explaining why require() wasn't used in this case?
|
this is really cool - and I could use this too, so yeah lets get this landed. I would like to use it from other rust apps at build time. |
|
@ndizazzo incredible feedback. I have done all of what you suggested and merged! |
* origin/main: Ship real client and serving SDKs across Swift, Kotlin, and Node.js (#634) task: Add tok/s and model name to nightly workflow summary (#654) fix(docs): restore docs.anarchai.org as Pages custom domain Update llama.cpp upstream pin Fix skippy smoke llama build directory (#655) Update pinned llama.cpp revision (#646) Normalize non-stream chat tool call IDs chore(github): Add lightweight issue templates to repo Add nightly mesh stability harness (#631) fix(moa): show real tok/s on mesh by reporting effective completion_tokens (#638)
3 participants
Users can now build MeshLLM applications against real SDK surfaces instead of examples or wrappers that only exercise partial behavior. This PR turns the SDK work into a usable client and serving stack across Swift, Kotlin, and Node.js, with model management, embedded serving load/unload, native runtime packaging, and CI checks to keep the contract aligned.
What changed
NodeAPI with client, model management, inference, and serving surfaces.MeshClient-style wrapper direction from Swift/Kotlin and aligned naming with language namespaces/modules.mesh-llm-*shape, includingmesh-llm-api,mesh-llm-ffi,mesh-llm-node, andmesh-llm-nodejs.libmeshllm_ffi/meshllm_ffi.dll.docs/SDK.mdfrom a planning/spec document into a usage guide for Swift, Kotlin, Node.js, native runtime artifacts, examples, lifecycle, errors, and platform support.User-facing SDK shape
The public SDK direction is now:
Nodeas the main entry point.node.modelsfor model search, metadata, installed models, downloads, and capabilities.node.inferencefor chat/responses/list/cancel.node.servingfor status, load, and unload.Native runtime packaging
This PR keeps
libllamabackend variants behind MeshLLM native runtime artifacts rather than making SDK consumers link directly to backend-specific llama libraries. SDK consumers load MeshLLM through the canonical native runtime library, and runtime artifacts carry the backend flavor metadata and checksum.Baseline artifact shapes include macOS Metal/CPU, Linux CPU/CUDA/Vulkan/ROCm, and Windows CPU/CUDA/Vulkan/ROCm.
Platform notes
Compatibility
This is SDK and packaging work. It does not intentionally change the mesh wire protocol. The public SDK API is being established here, so downstream Swift/Kotlin/Node consumers should move to the new
Nodesurfaces rather than the removed client wrapper names.Validation
Local validation run across this branch included:
cargo check -p mesh-llm-nodejscargo test -p mesh-llm-nodejscargo fmt --all -- --checknode --test sdk/node/test/*.test.jsscripts/check-sdk-contract.shgit diff --checkCI also has additional SDK contract, native runtime package, Swift/Kotlin smoke, Node addon, and Windows-relevant build paths so review catches drift before release.