feat(mtmd): multimodal prefill through the kernel + batched fan-out coverage

[lloyal-research]

Multimodal (vision) prefill: mmproj load, the embedding rail, and the branch fan-out on top of it.

Depends on lloyal-ai/liblloyal#36 — the submodule pointer here targets that branch and must be repointed at liblloyal main once it merges.

Why

e1fb5dc landed the feature with the mtmd pipeline inside the N-API worker: chunk walking, mtmd_encode_chunk, section-major position packing, per-segment dispatch and cell bookkeeping — with BranchState::position crossing the binding boundary. KV state belongs behind branch.hpp, and any other SessionContext implementation (Nitro/JSI, or a platform encoder like CoreML) would have had to reimplement the same walk.

What

The walk moved into liblloyal as BranchStore::decode_segments + MtmdSource. The worker now constructs a source and makes one kernel call:

lloyal::MtmdSource source(_mtmd, _prompts[i], _bitmapBytes[i],
    std::span<const llama_token>(_sepStorage[i]), _nEmbdInp);
const auto r = _store.decode_segments(_handles[i], source);

−142/+15 in the worker — the tell that the boundary is right. The CMake side is one block: llama.cpp does not build tools/mtmd under add_subdirectory (LLAMA_BUILD_TOOLS is off), so the addon adds that library itself, EXCLUDE_FROM_ALL.

Tests

Adds the batched fan-out case. The existing spine-share block proves forks over an image are free, but drives its two children sequentially with the same question. This drives four children together, each asking a different question, through the public BranchStore surface — which also covers the JS→N-API marshalling of parallel handle/token arrays that the kernel-level test in liblloyal#36 bypasses entirely.

[PASS] fan-out: fork ×4 over the image adds zero cells
[PASS] fan-out: costs only the text suffixes (96), never the image again
[PASS] fan-out: 16 batched dispatches, each carrying up to 4 branches
[PASS] "Describe the red object"  → "The red object is a solid square located in the upper left corner of the image"
[PASS] "Describe the blue object" → "The blue object is a solid circle located in the bottom right corner of the image"
[PASS] "what color is the square?" → "The square is red."
[PASS] "what color is the circle?" → "The circle is blue."
[PASS] fan-out: children release their own cells, image prefix intact

The children report spatial facts — upper-left vs bottom-right — which only hold if the M-RoPE positional grid survived the fork. Four branches reading different regions of one encode, sharing a KV prefix, decoded together.

Answers are deliberately short sentences rather than single words: a one-word answer emits its token and stops, so the loop would do a single dispatch and prove nothing about sustained batching. This is the difference between 1 tick and 16.

Suite: 267 passed, 0 failed.

Known unrelated failure

testRerankLargeCorpus fails on main and on this branch — the relevant document ranks 5th instead of top-3. It is pre-existing and unrelated: every function the reranker's text path executes (Scratch::as_batch, decode::scatter, decode::each, BranchStore::decode_scatter, decode_each) is byte-identical to HEAD in liblloyal#36, and the two that did change (release, retainOnly) only write the passive cells_used_ gauge.

Root cause measured: it stacks two lossy quant axes (q4_k_m weights × q4_0 KV), under which the pointwise judge returns negative for every document — the ranking is noise among rejects, not a near-miss. Fixed separately in #50.

[Copilot]

Pull request overview

This PR adds first-class multimodal (vision) prefill support to the Node addon by loading an mmproj (mtmd) context at createContext time and exposing a dedicated _storePrefillMultimodal entrypoint, alongside new integration coverage that validates batched fan-out behavior over a shared image prefix.

Changes:

• Build/link llama.cpp’s mtmd library and load mmproj via new createContext options (mmprojPath, imageMinTokens, imageMaxTokens).
• Add N-API surfaces for multimodal prefill and capability probes (_storePrefillMultimodal, supportsVision, supportsAudio), plus integration tests for sustained batched fan-out.
• Update CI model matrix/cache key and README documentation for multimodal usage.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/matrix.json	Adds SmolVLM + mmproj artifacts for multimodal CI tier.
test/integration.ts	Adds multimodal integration suite including batched fan-out coverage.
src/SessionContext.hpp	Declares mtmd ownership hook and new multimodal/probe methods.
src/SessionContext.cpp	Implements mmproj loading, mtmd lifecycle, and multimodal prefill worker + probes.
README.md	Documents multimodal setup and APIs; updates llama.cpp version badge.
CMakeLists.txt	Builds/links mtmd from llama.cpp tools and ships it with shared builds.
.github/workflows/release.yml	Bumps model cache key to include new multimodal CI models.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Suppressed comments (2)

src/SessionContext.cpp:2655

• The bitmap marshalling path accepts any TypedArray but treats its backing bytes as an image payload; this will silently accept non-Uint8 typed arrays (e.g. Int16Array) even though the error message says "Uint8Array". Restrict TypedArray inputs to Uint8Array (or update messaging) to avoid misinterpreting data.

      Napi::Value v = jsImgs.Get(j);
      if (!v.IsBuffer() && !v.IsTypedArray()) {
        throw Napi::Error::New(env,
          "_storePrefillMultimodal: bitmaps must be Buffer/Uint8Array");
      }

src/SessionContext.cpp:1941

• CreateContext allocates mtmdCtx (and ctx) before constructing the SessionContext JS object; if ctor.New({}) or unwrap throws, these native resources leak because they are not yet owned by SessionContext. Add an exception-safe guard (RAII or try/catch) to free mtmdCtx/ctx on any error before ownership transfer.

  // Create SessionContext instance
  Napi::Function ctor = env.GetInstanceData<Napi::FunctionReference>()->Value();
  Napi::Object instance = ctor.New({});
  SessionContext* obj = SessionContext::Unwrap(instance);

[lloyal-research]

Fixed in 6fde0ae — the snippet now creates the branch with _branchCreate before prefilling.

[lloyal-research]

Fixed in 6fde0ae, as a documentation note rather than a code change — this is a publish-ordering issue, not a defect. createContext is typed through @lloyal-labs/sdk's ContextOptions, and the installed SDK carries no mmprojPath; only the unreleased workspace copy does. The README now states that these options and members typecheck once an SDK carrying multimodal ContextOptions is installed, and that the runtime accepts them regardless. The real fix is publishing that SDK.

[Copilot]

🟡 Changes recommended

The new multimodal integration test’s local typing for _storePrefillMultimodal is inconsistent with how the test uses the returned error field, which is likely to break TypeScript compilation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 11/13 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

_storePrefillMultimodal’s duplicate-handle check compares DoubleValue() but later coerces handles via Uint32Value(), allowing duplicates to slip through and prefill the same branch twice in a single call.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/SessionContext.cpp:2783

• Duplicate-handle detection uses DoubleValue() and compares doubles, but the actual handles are later coerced with Uint32Value(). That means non-integer numbers (e.g. 1.2 and 1.7) can bypass the duplicate check while both coerce to the same uint32 handle, causing the same branch to be prefilled twice in one call.

    std::vector<double> seen;
    seen.reserve(n);
    for (uint32_t i = 0; i < n; i++) {
      const double h = jsHandles.Get(i).As<Napi::Number>().DoubleValue();
      for (uint32_t j = 0; j < seen.size(); j++) {

• Files reviewed: 11/13 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces substantial native binding + build-system changes (mtmd lifecycle, new async workers, and new external library target) that warrant final human review despite strong test additions.

Review details

• Files reviewed: 12/14 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Pending multimodal work can race context disposal, and several documentation and coverage assertions are currently inaccurate.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 13/15 changed files
• Comments generated: 5
• Review effort level: Balanced

[lloyal-research]

Real, and it is not specific to the new multimodal workers — every one of the 19 AsyncWorkers in SessionContext.cpp holds a raw llama_context/BranchStore/mtmd_context, and dispose()/~SessionContext free unconditionally with no in-flight tracking. Witnessed on real models: a pre-existing StorePrefillWorker (branch.prefill then dispose) SIGSEGVs, and CellsMultimodalWorker is killed. It is reachable above the binding because Effection 4.1.0 halt() returns with a call(nativePromise) still unsettled while initAgents runs ctx.dispose() on scope exit.

Scheduled as two units. The reachable case is closed on the runtime side first: hdk 7cda0ad (feat/mtmd) adds a waitUntilSettled combinator so the agent pool lets an in-flight decode settle before it prunes or disposes. This PR then gets the binding-level tripwire as its own follow-up after it merges: a SessionWorker base holding an ObjectReference and an in-flight count, with dispose()/_branchPrune/_branchFork/_storeRetainOnly refusing while work is in flight — fail loud rather than defer, since deferring hides the ordering bug. Not deferring the fix; deferring the second, binding-level half to its own change.

[lloyal-research]

Fixed in 64195a0. Confirmed against StoreCommitWorker (src/SessionContext.cpp:611-616): phase 2 calls accept_token for every item, so the README's Branch.accept() advanced grammar, sampler and metrics a second time. This predated the PR (it was on main at line 54); the SDK's own store loop never calls accept. Dropped it — commit does the accept once.

[lloyal-research]

Fixed in 64195a0. The test now records each dispatch width and asserts more than one dispatch carried several branches, so a lone survivor decoding serially no longer passes. On the real run the widths were 4,4,4,4,4,2,2,... — five full-width dispatches.

[lloyal-research]

Fixed in 64195a0. Verified: neither src/index.ts nor @lloyal-labs/sdk exports formatChat, parseChatOutput, jsonSchemaToGrammar or the metric functions as top-level values — they are methods on the context and branch (the native-surface block already shows them). The list now names what is actually re-exported (buildUserDelta, buildToolResultDelta, and the sampling/chat/rerank types).

[lloyal-research]

Fixed in 64195a0. testTokenizer now asserts a Buffer per token, that at least one piece ends mid-character, and that Buffer.concat equals detokenize(). Text chosen after surveying four tokenizers (SmolLM2, Llama-3.2, Phi-3.5, Qwen3.5 all tear); Phi keeps the SentencePiece prefix space on piece one, hence the "" + detok" arm. On SmolLM2: 34 of 36 pieces torn, concat equal.

[Copilot]

🔵 Needs a closer look

The new asynchronous multimodal workers can race with context disposal and dereference freed native state.

Review details

Suppressed comments (4)

README.md:57

• BranchStore.commit() already accepts each token before decoding it (see StoreCommitWorker in src/SessionContext.cpp:611-616). Calling Branch.accept() here accepts every sampled token twice, which can advance grammar/sampler/metrics state incorrectly. Pass the sampled pairs directly to store.commit().

    .map((p) => { p.b.accept(p.token); return [p.b, p.token]; });

test/integration.ts:2562

• ticks > 1 only proves that at least one child survives for a second dispatch; the other three may stop after the first tick. In the non-strict CI tier, the later checks require only one token per child, so this can pass without the sustained four-way batching the test is intended to cover. Record each entries.length and require at least two ticks with kids.length entries (or equivalently require every child to produce more than one token).

    assert(ticks > 1,
      `fan-out: ${ticks} batched dispatches, each carrying up to ${kids.length} branches`);

src/SessionContext.cpp:804

• This worker keeps raw references to the session's BranchStore and mtmd_context after returning control to JavaScript. ctx.dispose() can then synchronously drain the store and free both _mtmdContext and the llama context while Execute() is queued or running, causing a race/use-after-free during multimodal prefill. Keep the owning SessionContext alive and coordinate dispose() with in-flight workers (merely pinning the JS object would not prevent explicit disposal).

  lloyal::branch::BranchStore& _store;
  mtmd_context* _mtmd;
  int32_t _nEmbdInp;

src/SessionContext.cpp:2738

• _cellsMultimodal() also queues work with only a raw mtmd_context*. A caller can save the returned promise and immediately call ctx.dispose(), which frees that context before or during MtmdSource construction and leaves this worker dereferencing freed memory. Track this operation as in-flight and defer disposal/freeing until it completes, while retaining the owning session against GC.

private:
  Napi::Promise::Deferred _deferred;
  mtmd_context* _mtmd;
  int32_t _nEmbdInp;

• Files reviewed: 13/15 changed files
• Comments generated: 0 new
• Review effort level: Balanced