QVAC-22415 feat[api]: migrate @qvac/llm-llamacpp to the shared @qvac/fabric runtime - #4454
Merged
Merged
Conversation
Contributor
Review StatusCurrent Status: ❌ PENDING Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member. |
Contributor
License compliance — findings detected (warn-only)Critical: 0 · High: 2 · Medium: 0
How to resolve a blocking finding:
Warn-only (shadow) mode — this check does not block merges yet. Updated automatically by the canonical license compliance workflow. NOTICE presence (advisory)Missing NOTICE (advisory, does not block):
|
Contributor
Workflow security (shadow mode)zizmor found 1103 finding(s) in Findings are annotated inline on the changed files and listed in the job summary. Reproduce locally: pipx run zizmor==1.27.0 --offline .github/ |
jpgaribotti
had a problem deploying
to
release
September 14, 2026 14:31 — with 
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 14, 2026 14:34 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 14, 2026 14:34 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 14, 2026 14:34 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 14, 2026 14:34 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 14, 2026 14:48 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 14, 2026 14:48 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 15, 2026 08:20 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 15, 2026 08:20 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 15, 2026 08:20 — with
GitHub Actions
Failure
GitHub Actions
Failure
jpgaribotti
had a problem deploying
to
release
September 15, 2026 08:20 — with
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
…fabric runtime Moves the addon off its bundled, statically-linked qvac-fabric vcpkg build and onto the shared @qvac/fabric npm runtime. llama.cpp, ggml, mtmd and libcommon are now loaded once per process from the single @qvac/fabric install rather than duplicated inside every fabric consumer, which drops the addon binary from tens of MB to ~3.2 MB. This was the last Phase 1 addon on the vcpkg port, so no addon declares qvac-fabric any more -- packages/fabric is its only remaining consumer. - vcpkg.json drops qvac-fabric and the vk-profiling feature, along with the find_package(llama) and find_package(OpenSSL) calls the port required. Vulkan profiling is now a property of the shared runtime, chosen when building @qvac/fabric. - CMakeLists.txt adopts the shared cmake/qvac-addon template, replacing the hand-rolled preamble (triplet overlay, libc++ flags, lint-cpp config sync, Windows lean-header defines, --exclude-libs,ALL, JS_LOGGER/BACKENDS_SUBDIR and the manual GGML_AVAILABLE_BACKENDS staging loop). This also picks up the Android 16 KB page-size link flags and the Apple compiler-rt force_load the template applies everywhere. - binding.js preloads @qvac/fabric so the shared .bare module is registered before the addon resolves DT_NEEDED qvac__fabric@0.bare. - addon.ts resolves backendsDir from the @qvac/fabric install on desktop and falls back to the addon's own prebuilds on mobile, where the package tree is not resolvable from the packed worklet bundle. BACKENDS_SUBDIR moves from <host>/llm-llamacpp to <host>/qvac__fabric to match. - Finetuning keeps its API but now resolves llama_opt_* / ggml_opt_* and common_opt_sft_dataset_init from the shared runtime instead of from objects linked into this addon. - C++ tests and the optional BUILD_CLI tool link the fabric headers and module instead of llama directly. Neither gets the wiring add_bare_module() gives the .bare module, so both stage qvac__fabric and its dlopen'd backends beside the binary and rpath them in. backend_env.cpp preloads those backends from GGML_BACKEND_DIR -- without it no backend registers, not even CPU, and tests abort on a null deref under ASan. - run-cpp-tests.js owns the ASan relaxations required by linking a non-ASan, -static-libstdc++ fabric prebuild, so a local run matches CI instead of depending on the workflow to export them. Covered by unit tests. - CI workflows join the fabric-stack flow: llm is added to the npm_runtime consumers and the on-pr workflow detects fabric-stack PRs and threads a fabric-overlay-artifact through the cpp-test, prebuild and integration lanes. Verified locally: the built module carries DT_NEEDED qvac__fabric@0.bare, and of its undefined symbols none in the llama_* / ggml_* / gguf_* / mtmd_* / common_* / string_* / json_schema_* families are missing from the fabric prebuild. The addon loads under Bare, the C++ suite runs 160 tests green under ASan, cli_tool executes into the shared runtime, and the JS unit suites pass.
….53.2 The migration commit was written against fabric 0.13.0, before the C++ runtime work landed. The floor is now ^0.16.0: that is the release whose version node the addon pins its runtime to, and building against anything earlier either resolves the runtime from the host's libstdc++ or fails the template's assertion outright. Released as a patch. Migrating off the vcpkg qvac-fabric port changes how the addon is packaged and loaded, not what it exposes, so consumers on the 0.53.x line -- sdk, inference and rag all carry ^0.53.0 -- pick it up without a range change of their own and stay workspace-linked. A minor would have unlinked all three and, until 0.53.2 published, failed the ts-checks lane, which installs standalone from the public registry. The changelog entry moves with it. Rebasing put the migration's section mid-file as a second [0.53.0], since main had released that version meanwhile; it is now [0.53.2] at the top, with the runtime pin and the export narrowing described as part of the migration rather than as a follow-up, because for this addon they arrive together. The lockfile also relinks @qvac/asr-ggml, which main records against the registry although the workspace version satisfies the range. Pre-existing drift, corrected by regenerating rather than introduced here.
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
iancris
reviewed
Sep 17, 2026
Without the gate, a fabric-stack PR would lint this addon against published fabric headers while every other native lane built against the PR's runtime.
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
GustavoA1604
previously approved these changes
Sep 17, 2026
0.16.0 stamped its named ELF version node on every target, including Android, where fabric exports no C++ ABI to pin to. A module built against it recorded the version need anyway and failed its dlopen on device, so every addon reported ADDON_NOT_FOUND before any model work. 0.16.1 (#4566) ships that node anonymous on the links that export no runtime. ^0.16.0 still admits the broken release, which matters for a source build on Android; the six other fabric consumers already moved to ^0.16.1 on main with their patch releases. Rebuilt against 0.16.1, this addon carries no fabric version needs at all and the full Android suite passes 53/53 on Pixel 9 and Galaxy S25 Ultra. Lockfile regenerated with the pinned pnpm: the workspace fabric is 0.16.1, so the only change is the specifier and the link to it is preserved.
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
Without the input, a fabric-stack PR would run llm cpp-tests and integration through on-pr-nx against published fabric while the bespoke on-pr path used the PR runtime. Also record the ^0.16.1 floor in the addon docs.
gianni-cor
approved these changes
Sep 18, 2026
iancris
approved these changes
Sep 18, 2026
GustavoA1604
approved these changes
Sep 18, 2026
Contributor
🧪 C++ Test Coverage ReportCoverage: 📊 Detailed Coverage |
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
4 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.
🎯 What problem does this PR solve?
@qvac/llm-llamacppstatically linked theqvac-fabricvcpkg port, so every fabric consumer in a process carried its own copy of llama.cpp + ggml + mtmd. This was the last Phase 1 addon still on the port — after this, no addon declaresqvac-fabricandpackages/fabricis its only remaining consumer.📝 How does it solve it?
vcpkg.jsondropsqvac-fabricand thevk-profilingfeature, plus thefind_package(llama)/find_package(OpenSSL)calls the port required. Vulkan profiling is now a property of the shared runtime, chosen when building@qvac/fabric. Remaining vcpkg deps:picojson,nlohmann-json,concurrentqueue,qvac-lib-inference-addon-cpp,qvac-lint-cpp.CMakeLists.txtadopts the sharedcmake/qvac-addontemplate, replacing the hand-rolled preamble (triplet overlay, libc++ flags, lint-cpp config sync, Windows lean-header defines,--exclude-libs,ALL,JS_LOGGER/BACKENDS_SUBDIR, and the manualGGML_AVAILABLE_BACKENDSstaging loop). It also picks up the Android 16 KB page-size link flags and the Apple compiler-rtforce_loadthe template applies everywhere.binding.jspreloads@qvac/fabricso the shared.baremodule is registered before the addon resolvesDT_NEEDED qvac__fabric@0.bare.addon.tsresolvesbackendsDirfrom the@qvac/fabricinstall on desktop, falling back to the addon's ownprebuilds/on mobile where the package tree isn't resolvable from the packed worklet bundle.BACKENDS_SUBDIRmoves from<host>/llm-llamacppto<host>/qvac__fabric.llama_opt_*/ggml_opt_*/common_opt_sft_dataset_initfrom the shared runtime instead of objects linked into this addon.BUILD_CLItool link the fabric headers and module instead of llama directly. Neither gets the wiringadd_bare_module()gives the.baremodule, so both stageqvac__fabricand itsdlopen'd backends beside the binary and rpath them in. A newbackend_env.cpppreloads those backends fromGGML_BACKEND_DIR— without it no backend registers, not even CPU, and tests abort on a null deref under ASan.run-cpp-tests.jsowns the ASan relaxations required by linking a non-ASan,-static-libstdc++fabric prebuild, so a local run matches CI rather than depending on the workflow to export them. Covered by new unit tests.npm_runtimein.github/fabric-consumers.json, andon-pr-llm-llamacpp.ymldetects fabric-stack PRs and threads afabric-overlay-artifactthrough the cpp-test, prebuild and integration lanes. This wiring was diffed againston-pr-embed-llamacpp.ymland is semantically identical.docs/architecture/ADDON-CMAKE-TEMPLATE.mdis updated to record Phase 1 as complete, since this PR closes it out.🧪 How was it tested?
CI
Dispatched
on-pr-llmrun on the rebased head (fa39cb01).detect-fabric-stacktreats a dispatch as a stack run, butneeds_fabric_artifactispull_request_target-only, so every native job here built against the released@qvac/fabric0.16.0 from npm — the state this PR merges into.linux-x64,linux-arm64,darwin-x64,darwin-arm64,win32-x64,android-arm64,ios-arm64,ios-arm64-simulator,ios-x64-simulator).linux-x64:qvac-addon: verifying qvac__llm-llamacpp-0.53.2-eb99933f imports fabric's C++ runtime. The check is Linux-only by design and is skipped on Android, where fabric does not own the runtime.Unknown errorbefore the@qvac/fabric0.16.0 runtime pin:linux-x64qvac-ubuntu2404-x64-gpulinux-x64qvac-ubuntu2204-x64-gpu-runner2linux-arm64ubuntu-24.04-armlinux-arm64ubuntu-22.04-armdarwin-arm64qvac-macos26-arm64-gpu2darwin-x64macos-15win32-x64qvac-win25-x64-gpu2cpp-lint,sanity-checks,verify-fabric-lockstep,detect-native-changes, prebuild merge and artifact save.Two lanes the run does not cover:
cpp-testsnever started, which is the only reason the run is red — no job was created for it, and every other job is green or skipped. This is pre-existing onmainrather than something this PR introduces:cpp-tests-llm.ymlis the onlycpp-tests-*workflow carrying aconcurrencyblock (added by QVAC-24194 infra: make the llm cpp-tests vcpkg cache work #4103), and its group expression is identical to its caller's.github.workflowinside a reusable workflow resolves to the calling workflow's name, so the nested call asks for the group its own caller already holds and GitHub schedules nothing. Corroborated three ways: across the last 25on-pr-llmruns the nestedcpp-testshas never once run (skipped where the label was absent, missing where it was enabled, and the run red every time it was missing); the same commit'son-pr-embedrun, whose reusable workflow has noconcurrencyblock, ran its five C++ legs green; and dispatchingcpp-tests-llm.ymldirectly does create its jobs. The C++ suite for this branch was therefore validated locally (below), and the workflow fix belongs in its own PR since it affects every LLM PR.pull_request_targetrun of a PR that actually changespackages/fabricwith theprebuildslabel. This PR does not touchpackages/fabric, so the wiring was diffed againston-pr-embed-llamacpp.ymlinstead of executed. Mobile device-farm suites are likewise on-demand only; both were dispatched for this branch and are reported below.Mobile (Device Farm)
Both suites ran against GPR dev builds of this branch, published by
on-merge-llm-llamacpp.ymlfromtmp-QVAC-22415(this head, minus one CI-only commit that toucheson-pr-llm-llamacpp.yml).0.53.2-tmp.runid-353271632880.53.2-tmp.runid-35257060208@qvac/fabric0.16.1 (QVAC-25104 fix: name fabric's ELF version node only where it owns the runtime #4566), not the 0.16.0 the CI run above built against. The first Android dispatch (35257948413, on the 0.16.0 build) failed every test at addon load withADDON_NOT_FOUND: Cannot find addon '.', before any model work. 0.16.0 named its ELF version node on every target, so this addon recorded aDT_VERNEEDonQVAC_FABRIC_ABI_1with 153 pinned imports, and itsdlopenfailed on device. Not specific to this PR: published@qvac/embed-llamacpp0.41.1 (fabric 0.15.0) passes the Android suite while 0.41.2 (fabric 0.16.0, no change to its load path) fails identically. 0.16.1 ships that node anonymous again on the links that export no C++ ABI; rebuilt against it, this addon carries no fabric version needs at all and the suite is green.if(UNIX AND NOT APPLE), and the Apple link takes-exported_symbols_listagainstexports.txt— Mach-O has no symbol versioning for a version node to exist in, andsymbols.mapnever reaches it.^0.16.1, matching the six other fabric consumers onmain.^0.16.0would still admit the broken release, which a source build on Android would reproduce.Locally on
linux-x64DT_NEEDEDconfirmed on the built module:llama_* / ggml_* / gguf_* / mtmd_* / common_* / string_* / json_schema_*families are missing from the fabric prebuild — including the finetuning training API, which was the main open question.createInstanceresolves), andbackendsDirresolves to the fabric install with all 16 ggml backend modules present.addon-testbuilds, stagesqvac__fabric@0.bare+ backends,lddresolves via$ORIGIN, and 160 tests pass green under ASan (23 skipped — they need model fixtures not downloadable in this environment).cli_toolbuilds, links fabric, and executes into the shared runtime's GGUF parser.npm run lint,typecheck, andtest:unitall pass (108/108 bare tests, 90/90 node tests, mobile manifest validations).💥 Breaking Changes
None for API consumers. Two build/packaging notes for anyone consuming this package directly:
@qvac/fabriccarries the prebuilt runtime inside its own tarball, sonpm installmust run beforebare-make generate/build, and the dependency must not be pruned at runtime.vk-profilingbuild feature and theVK_PROFILINGCMake option / prebuild input are gone; select Vulkan profiling when building@qvac/fabricinstead.