Skip to content

Commit 3fd25b2

Browse files
committed
infra: align embed-llamacpp integration model helpers with diffusion-cpp
Make the integration-test model tooling uniform with diffusion-cpp: - ensureModel now takes an object ({ modelName, modelDir, manifest, download }), matching diffusion-cpp/llm-llamacpp; update all callsites - log the "no sha256/bytes pinned — integrity check SKIPPED" hint pointing at scripts/generate-model-manifest.mjs, same as diffusion - rename the pinning script to scripts/generate-model-manifest.mjs (verbatim diffusion copy) and the unit suite to test/unit/ensure-model-integrity.test.js - manifest integrity stays null until pinned in CI (HF_TOKEN + network); the suite tolerates null now and enforces shape once pinned Ref: QVAC-21937
1 parent 2f81230 commit 3fd25b2

5 files changed

Lines changed: 64 additions & 35 deletions

File tree

packages/embed-llamacpp/scripts/pin-model-manifest.mjs renamed to packages/embed-llamacpp/scripts/generate-model-manifest.mjs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,13 @@
44
// Populates sha256 + bytes in test/integration/models.manifest.json by
55
// downloading each pinned URL FRESH into a temp directory and hashing it.
66
//
7-
// This is the integration-test model cache manifest (single source of truth for
8-
// URLs + integrity, and the .github/actions/cache-models cache key).
9-
//
107
// IMPORTANT (integrity provenance): shas are computed from a clean download of
118
// the pinned URL, never from packages/embed-llamacpp/test/model (which may be a
129
// stale or poisoned restored cache). Run this on a machine/CI with network and
1310
// an HF_TOKEN for gated repos.
1411
//
1512
// Usage:
16-
// HF_TOKEN=hf_xxx node scripts/pin-model-manifest.mjs [--only <modelName>] [--force]
13+
// HF_TOKEN=hf_xxx node scripts/generate-model-manifest.mjs [--only <modelName>] [--force]
1714
//
1815
// Flags:
1916
// --only <name> Only (re)generate the named model entry.

packages/embed-llamacpp/test/integration/addon.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ const DEVICE_API = isDarwinX64 || isLinuxArm64 ? 'cpu' : 'gpu'
592592
const MODEL_NAME_API = getModelConfigs()[0]?.modelName ?? 'embeddinggemma-300M-Q8_0.gguf'
593593

594594
async function setupModelApiBehavior (t) {
595-
await ensureModel(MODEL_NAME_API)
595+
await ensureModel({ modelName: MODEL_NAME_API })
596596
const { inference } = await createEmbeddingsTestInstance(
597597
t,
598598
MODEL_NAME_API,

packages/embed-llamacpp/test/integration/multi-gpu.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ async function runMultiGpuTest (t, extraConfig, assertDevices) {
3535
return
3636
}
3737

38-
const [modelName, dirPath] = await ensureModel(MODEL.name)
38+
const [modelName, dirPath] = await ensureModel({ modelName: MODEL.name })
3939
const modelPath = path.join(dirPath, modelName)
4040
const specLogger = attachSpecLogger({ forwardToConsole: true })
4141

packages/embed-llamacpp/test/integration/utils.js

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -301,10 +301,12 @@ function prestagedModelDir (modelName) {
301301

302302
/**
303303
* Ensures the model file exists, downloading it if necessary
304-
* @param {string} modelName - The model name to ensure
304+
* @param {Object} opts
305+
* @param {string} opts.modelName - The model name to ensure
305306
* @returns {Promise<[string, string]>} Returns [modelName, modelDir]
306307
*/
307-
async function ensureModel (modelName, { modelDir = path.resolve(__dirname, '../model'), manifest, download } = {}) {
308+
async function ensureModel ({ modelName, modelDir, manifest, download } = {}) {
309+
const dir = modelDir || path.resolve(__dirname, '../model')
308310
const modelConfig = getModelConfig(modelName)
309311

310312
// Model URL + sha256/bytes come from models.manifest.json (by modelName); the
@@ -318,7 +320,7 @@ async function ensureModel (modelName, { modelDir = path.resolve(__dirname, '../
318320
throw new Error(`Unknown model: ${modelName}`)
319321
}
320322

321-
const modelPath = path.join(modelDir, modelName)
323+
const modelPath = path.join(dir, modelName)
322324
const doDownload = download || downloadFileWithRetries
323325
const urls = hasManifestUrls ? entry.urls : [modelConfig.downloadUrl]
324326
const hasIntegrity = entryHasIntegrity(entry)
@@ -328,17 +330,21 @@ async function ensureModel (modelName, { modelDir = path.resolve(__dirname, '../
328330
const res = await verifyModelFile(modelPath, entry)
329331
if (res.ok) {
330332
console.log(`[download] ${modelName}: cached copy verified, skipping download`)
331-
return [modelName, modelDir]
333+
return [modelName, dir]
332334
}
333335
console.log(`[download] ${modelName}: cached copy failed integrity (${res.reason}); deleting and re-downloading`)
334336
try { fs.unlinkSync(modelPath) } catch (_) {}
335337
} else {
336338
const stat = fs.statSync(modelPath)
337-
if (stat.size > 0) {
338-
return [modelName, modelDir]
339+
if (stat.size === 0) {
340+
console.log(`[download] Removing zero-byte cached file: ${modelName}`)
341+
fs.unlinkSync(modelPath)
342+
} else {
343+
if (entry) {
344+
console.log(`[download] ${modelName}: no sha256/bytes pinned in manifest — integrity check SKIPPED (run scripts/generate-model-manifest.mjs to pin)`)
345+
}
346+
return [modelName, dir]
339347
}
340-
console.log(`[download] Removing zero-byte cached file: ${modelName}`)
341-
fs.unlinkSync(modelPath)
342348
}
343349
}
344350

@@ -348,18 +354,18 @@ async function ensureModel (modelName, { modelDir = path.resolve(__dirname, '../
348354
// load() writes sibling files next to the model (e.g. openclCacheDir).
349355
const staged = prestagedModelDir(modelName)
350356
if (staged) {
351-
fs.mkdirSync(modelDir, { recursive: true })
357+
fs.mkdirSync(dir, { recursive: true })
352358
console.log(`[prestage] Using pre-staged model ${modelName} (copying into writable modelDir)`)
353359
fs.copyFileSync(path.join(staged, modelName), modelPath)
354360
const stat = fs.statSync(modelPath)
355361
if (stat.size === 0) {
356362
fs.unlinkSync(modelPath)
357363
throw new Error(`[prestage] copied model ${modelName} is empty`)
358364
}
359-
return [modelName, modelDir]
365+
return [modelName, dir]
360366
}
361367

362-
fs.mkdirSync(modelDir, { recursive: true })
368+
fs.mkdirSync(dir, { recursive: true })
363369
console.log(`[download] Downloading test model: ${modelName}...`)
364370
_downloadCount++
365371

@@ -376,7 +382,7 @@ async function ensureModel (modelName, { modelDir = path.resolve(__dirname, '../
376382
const stat = fs.statSync(modelPath)
377383
console.log(`[download] Model ready: ${(stat.size / 1024 / 1024).toFixed(1)}MB`)
378384

379-
return [modelName, modelDir]
385+
return [modelName, dir]
380386
}
381387

382388
/**
@@ -410,7 +416,7 @@ class TestLogger {
410416
* @returns {Promise<{inference: GGMLBert}>}
411417
*/
412418
async function createEmbeddingsTestInstance (t, modelName, device = 'gpu', gpuLayers = null, batchSize = '1024') {
413-
const [, modelDir] = await ensureModel(modelName)
419+
const [, modelDir] = await ensureModel({ modelName })
414420
const modelPath = path.join(modelDir, modelName)
415421

416422
t.ok(fs.existsSync(modelPath), 'Model file should exist')

packages/embed-llamacpp/test/unit/ensure-model-manifest.test.js renamed to packages/embed-llamacpp/test/unit/ensure-model-integrity.test.js

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
'use strict'
22

3-
// Offline proof for the models.manifest.json wiring (QVAC-21937): ensureModel
4-
// resolves URLs + integrity from the manifest, re-downloads poisoned/truncated
3+
// Offline integrity/manifest proof for embed-llamacpp, mirroring
4+
// diffusion-cpp/test/unit/ensure-model-integrity.test.js: ensureModel resolves
5+
// URL + sha256/bytes from models.manifest.json, re-downloads poisoned/truncated
56
// caches, falls back to MODEL_CONFIGS when the manifest lacks an entry, and the
6-
// real integration manifest is well-formed. No network and no native addon are
7-
// touched — downloads are injected.
7+
// real manifest is well-formed. No network and no native addon are touched —
8+
// downloads are injected.
89

910
const test = require('brittle')
1011
const fs = require('bare-fs')
@@ -27,7 +28,7 @@ const BAD_SHORT = 'too-short'
2728

2829
function mkTmpDir () {
2930
const base = (typeof os.tmpdir === 'function' && os.tmpdir()) || '/tmp'
30-
const dir = path.join(base, `qvac-embed-manifest-${Date.now()}-${Math.random().toString(36).slice(2)}`)
31+
const dir = path.join(base, `qvac-embed-integrity-${Date.now()}-${Math.random().toString(36).slice(2)}`)
3132
fs.mkdirSync(dir, { recursive: true })
3233
return dir
3334
}
@@ -80,7 +81,7 @@ test('correct cached file verifies and performs NO download', async function (t)
8081
try {
8182
writeModel(dir, name, GOOD)
8283
resetDownloadCount()
83-
const [, resolvedDir] = await ensureModel(name, { modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
84+
const [, resolvedDir] = await ensureModel({ modelName: name, modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
8485
t.is(spy.calls, 0, 'no download for a valid cached file')
8586
t.is(getDownloadCount(), 0, 'download counter stays 0 on a warm run')
8687
t.is(resolvedDir, dir)
@@ -95,12 +96,12 @@ test('poisoned cached file (sha mismatch) is deleted and re-downloaded', async f
9596
const dir = mkTmpDir()
9697
const spy = { calls: 0 }
9798
try {
98-
writeModel(dir, name, BAD_SAME_LEN)
99+
writeModel(dir, name, BAD_SAME_LEN) // same length -> defeats size check, forces sha path
99100
resetDownloadCount()
100-
await ensureModel(name, { modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
101+
await ensureModel({ modelName: name, modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
101102
t.is(spy.calls, 1, 're-downloaded exactly once after integrity failure')
102103
t.is(getDownloadCount(), 1, 'download counter incremented')
103-
t.is(fs.readFileSync(path.join(dir, name), 'utf8'), GOOD)
104+
t.is(fs.readFileSync(path.join(dir, name), 'utf8'), GOOD, 'file replaced with correct content')
104105
} finally {
105106
fs.rmSync(dir, { recursive: true, force: true })
106107
}
@@ -113,7 +114,7 @@ test('truncated cached file (size mismatch) is re-downloaded', async function (t
113114
const spy = { calls: 0 }
114115
try {
115116
writeModel(dir, name, BAD_SHORT)
116-
await ensureModel(name, { modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
117+
await ensureModel({ modelName: name, modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
117118
t.is(spy.calls, 1, 'size mismatch triggered a re-download')
118119
t.is(fs.readFileSync(path.join(dir, name), 'utf8'), GOOD)
119120
} finally {
@@ -129,7 +130,7 @@ test('persistent mismatch hard-fails and removes the bad file', async function (
129130
try {
130131
writeModel(dir, name, BAD_SAME_LEN)
131132
await t.exception(
132-
ensureModel(name, { modelDir: dir, manifest, download: fakeDownloader(BAD_SAME_LEN, spy) }),
133+
ensureModel({ modelName: name, modelDir: dir, manifest, download: fakeDownloader(BAD_SAME_LEN, spy) }),
133134
/failed integrity/
134135
)
135136
t.is(spy.calls, 1, 'attempted a re-download before failing')
@@ -145,21 +146,35 @@ test('missing file downloads then verifies', async function (t) {
145146
const dir = mkTmpDir()
146147
const spy = { calls: 0 }
147148
try {
148-
await ensureModel(name, { modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
149+
await ensureModel({ modelName: name, modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
149150
t.is(spy.calls, 1, 'downloaded the missing file')
150151
t.is(fs.readFileSync(path.join(dir, name), 'utf8'), GOOD)
151152
} finally {
152153
fs.rmSync(dir, { recursive: true, force: true })
153154
}
154155
})
155156

157+
test('entry without pinned integrity skips verification (keeps a non-zero cached file)', async function (t) {
158+
const name = 'gte-large_fp16.gguf'
159+
const manifest = { models: { [name]: { urls: ['https://example.invalid/x'], sha256: null, bytes: null } } }
160+
const dir = mkTmpDir()
161+
const spy = { calls: 0 }
162+
try {
163+
writeModel(dir, name, BAD_SAME_LEN) // any non-zero content is accepted when unpinned
164+
await ensureModel({ modelName: name, modelDir: dir, manifest, download: fakeDownloader(GOOD, spy) })
165+
t.is(spy.calls, 0, 'no download when an unpinned file already exists and is non-zero')
166+
} finally {
167+
fs.rmSync(dir, { recursive: true, force: true })
168+
}
169+
})
170+
156171
test('MODEL_CONFIGS is used as a fallback when the manifest has no entry', async function (t) {
157172
const name = 'gte-large_fp16.gguf' // known in MODEL_CONFIGS
158173
const dir = mkTmpDir()
159174
const spy = { calls: 0 }
160175
try {
161176
t.ok(MODEL_CONFIGS[name], 'model is declared in MODEL_CONFIGS')
162-
await ensureModel(name, { modelDir: dir, manifest: { models: {} }, download: fakeDownloader(GOOD, spy) })
177+
await ensureModel({ modelName: name, modelDir: dir, manifest: { models: {} }, download: fakeDownloader(GOOD, spy) })
163178
t.is(spy.calls, 1, 'downloaded via the MODEL_CONFIGS fallback url')
164179
t.is(fs.readFileSync(path.join(dir, name), 'utf8'), GOOD)
165180
} finally {
@@ -171,7 +186,7 @@ test('truly unknown model (no config, no manifest entry) throws', async function
171186
const dir = mkTmpDir()
172187
try {
173188
await t.exception(
174-
ensureModel('nope.gguf', { modelDir: dir, manifest: { models: {} } }),
189+
ensureModel({ modelName: 'nope.gguf', modelDir: dir, manifest: { models: {} } }),
175190
/Unknown model/
176191
)
177192
} finally {
@@ -193,17 +208,28 @@ test('verifyModelFile flags size before hashing (fail-fast)', async function (t)
193208
}
194209
})
195210

196-
test('real integration manifest is well-formed and matches MODEL_CONFIGS', function (t) {
211+
test('integration manifest is well-formed and covers MODEL_CONFIGS', function (t) {
197212
const manifest = loadManifest()
198213
t.ok(manifest && manifest.models, 'models.manifest.json loads')
199214
const names = Object.keys(manifest.models)
200215
t.is(new Set(names).size, names.length, 'no duplicate model keys')
216+
201217
for (const [name, entry] of Object.entries(manifest.models)) {
202218
const hasUrl = Array.isArray(entry.urls) && entry.urls.length > 0 &&
203219
entry.urls.every(u => typeof u === 'string' && u.startsWith('https://'))
204220
t.ok(hasUrl, `${name} has at least one https url`)
221+
222+
// Integrity is pinned in CI (scripts/generate-model-manifest.mjs). When
223+
// present it must be the right shape; null is allowed until pinned.
224+
if (entry.sha256 !== null && entry.sha256 !== undefined) {
225+
t.ok(/^[0-9a-f]{64}$/i.test(entry.sha256), `${name} sha256 is 64 hex chars when pinned`)
226+
}
227+
if (entry.bytes !== null && entry.bytes !== undefined) {
228+
t.ok(Number.isInteger(entry.bytes) && entry.bytes > 0, `${name} bytes is a positive integer when pinned`)
229+
}
205230
}
206-
// Every MODEL_CONFIGS model must be represented in the manifest so warm covers it.
231+
232+
// Every MODEL_CONFIGS model must be represented so warm covers it.
207233
for (const name of Object.keys(MODEL_CONFIGS)) {
208234
t.ok(resolveModelEntry(name), `${name} present in manifest`)
209235
}

0 commit comments

Comments
 (0)