From 711f32e86e7eb2b8113440ef9ef8ecb14fde0f9e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 02:35:23 -0400 Subject: [PATCH 1/2] Fix review findings: exact measurement surfaces, symmetric model matching, wrong-shape tags = unobservable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Upstream layer (embedTexts/upsertPoints/searchPoints/ensureCollection) now RETURNS the exact URL it fetched as `surface`; app.js reports it verbatim so measurement_surface can never drift from the real call (encoded collection names, ?wait=true included). - Embed-model presence matching is now symmetric on name:tag — base-name match when either side lacks a tag, exact tag match when both carry one. - A 200 /api/tags response that parses but has no models array is now unobservable, never absent: only a readable models list may prove absence. - Tests: tag matching both directions + tag mismatch + base-name mismatch, wrong-shape/non-JSON tags responses, and surface exactness asserted against the mocked fetch's actual URL (including URL-encoded collection names). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012T1XF1ZyJL7KW8AVRUJjiD --- src/app.js | 70 ++++++++++++++---- src/upstreams.js | 29 ++++++-- test/app.test.js | 189 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 261 insertions(+), 27 deletions(-) diff --git a/src/app.js b/src/app.js index 74d2846..890142c 100644 --- a/src/app.js +++ b/src/app.js @@ -8,6 +8,30 @@ import { searchPoints, } from "./upstreams.js"; +/** + * Split an Ollama model name into { base, tag }. The tag is whatever follows + * the LAST colon (names may contain '/' and registry paths); no colon => no tag. + */ +function splitModelName(name) { + const idx = name.lastIndexOf(":"); + if (idx === -1) return { base: name, tag: null }; + return { base: name.slice(0, idx), tag: name.slice(idx + 1) }; +} + +/** + * Symmetric embed-model matching: base names must match; when BOTH sides carry + * a tag the tags must match exactly; when EITHER side has no tag, the base-name + * match suffices (nomic-embed-text ~ nomic-embed-text:latest, both directions). + */ +function modelNamesMatch(listed, wanted) { + if (typeof listed !== "string" || typeof wanted !== "string") return false; + const a = splitModelName(listed); + const b = splitModelName(wanted); + if (a.base !== b.base) return false; + if (a.tag !== null && b.tag !== null) return a.tag === b.tag; + return true; +} + /** * Build the express app. `fetchImpl` is injectable so unit tests run without * Qdrant/Ollama up. @@ -38,21 +62,31 @@ export function createApp(config, { fetchImpl = fetch } = {}) { classify: (_res, bodyText) => { // While we can see the tag list, also classify the embed model: // present -> ok, listed-but-missing -> absent (a true gap, distinct - // from "could not look"). + // from "could not look"). ONLY a genuinely readable models list may + // prove absence — a parseable-but-wrong-shape response is + // unobservable, never absent. + let parsed; try { - const models = JSON.parse(bodyText)?.models ?? []; - const found = models.some( - (m) => m?.name === config.embedModel || m?.name?.split(":")[0] === config.embedModel - ); - embedModelResult = probeResult(found ? OK : ABSENT, ollamaSurface, { - model: config.embedModel, - }); + parsed = JSON.parse(bodyText); } catch { embedModelResult = probeResult(UNOBSERVABLE, ollamaSurface, { model: config.embedModel, reason: "tag list unparseable", }); + return null; } + const models = parsed?.models; + if (!Array.isArray(models)) { + embedModelResult = probeResult(UNOBSERVABLE, ollamaSurface, { + model: config.embedModel, + reason: "tags response parsed but carries no models array", + }); + return null; + } + const found = models.some((m) => modelNamesMatch(m?.name, config.embedModel)); + embedModelResult = probeResult(found ? OK : ABSENT, ollamaSurface, { + model: config.embedModel, + }); return null; // do not override the reachability verdict }, }), @@ -97,11 +131,13 @@ export function createApp(config, { fetchImpl = fetch } = {}) { return res.status(400).json({ error: "body must be {texts: string[]} with at least one text" }); } try { - const vectors = await embedTexts(texts, upstreamOpts); + const { vectors, surface } = await embedTexts(texts, upstreamOpts); res.json({ vectors, model: config.embedModel, - measurement_surface: `${config.ollamaUrl}/api/embeddings`, + // Reported verbatim from the upstream layer — byte-exact with the URL + // actually fetched, so it can never drift from the real call. + measurement_surface: surface, observed_at: new Date().toISOString(), }); } catch (err) { @@ -124,19 +160,20 @@ export function createApp(config, { fetchImpl = fetch } = {}) { return res.status(400).json({ error: "body must include items: [{id, text, payload?}] with at least one item" }); } try { - const vectors = await embedTexts(items.map((it) => it.text), upstreamOpts); + const { vectors } = await embedTexts(items.map((it) => it.text), upstreamOpts); await ensureCollection(collection, vectors[0].length, upstreamOpts); const points = items.map((it, i) => ({ id: it.id, vector: vectors[i], payload: { ...(it.payload ?? {}), text: it.text }, })); - const result = await upsertPoints(collection, points, upstreamOpts); + const { result, surface } = await upsertPoints(collection, points, upstreamOpts); res.json({ indexed: items.length, collection, qdrant: result?.result ?? result, - measurement_surface: `${config.qdrantUrl}/collections/${collection}/points`, + // Verbatim from the upstream layer: encoded collection + ?wait=true. + measurement_surface: surface, observed_at: new Date().toISOString(), }); } catch (err) { @@ -153,13 +190,14 @@ export function createApp(config, { fetchImpl = fetch } = {}) { } const lim = Number.isInteger(limit) && limit > 0 ? Math.min(limit, 100) : 10; try { - const [vector] = await embedTexts([query], upstreamOpts); - const result = await searchPoints(collection, vector, lim, upstreamOpts); + const { vectors: [vector] } = await embedTexts([query], upstreamOpts); + const { result, surface } = await searchPoints(collection, vector, lim, upstreamOpts); res.json({ results: result.map((r) => ({ id: r.id, score: r.score, payload: r.payload ?? null })), collection, limit: lim, - measurement_surface: `${config.qdrantUrl}/collections/${collection}/points/search`, + // Verbatim from the upstream layer (encoded collection name). + measurement_surface: surface, observed_at: new Date().toISOString(), }); } catch (err) { diff --git a/src/upstreams.js b/src/upstreams.js index 68e5f13..6954523 100644 --- a/src/upstreams.js +++ b/src/upstreams.js @@ -42,7 +42,12 @@ async function postJson(url, body, { fetchImpl, timeoutMs }) { return json; } -/** Embed each text via Ollama's embeddings API. Returns number[][]. */ +/** + * Embed each text via Ollama's embeddings API. + * Returns { vectors: number[][], surface } where `surface` is the exact URL + * that was fetched — callers MUST report that value (never rebuild it) so the + * reported measurement_surface can never drift from the real call. + */ export async function embedTexts(texts, { ollamaUrl, embedModel, fetchImpl = fetch, timeoutMs = 30000 }) { const url = `${ollamaUrl}/api/embeddings`; const vectors = []; @@ -53,7 +58,7 @@ export async function embedTexts(texts, { ollamaUrl, embedModel, fetchImpl = fet } vectors.push(json.embedding); } - return vectors; + return { vectors, surface: url }; } /** Ensure a Qdrant collection exists with the given vector size (Cosine). */ @@ -65,7 +70,7 @@ export async function ensureCollection(collection, vectorSize, { qdrantUrl, fetc } catch (err) { throw new UpstreamError(`request failed: ${err?.message}`, { surface: checkUrl, cause: err }); } - if (res.ok) return { created: false }; + if (res.ok) return { created: false, surface: checkUrl }; if (res.status !== 404) { throw new UpstreamError(`HTTP ${res.status} checking collection`, { surface: checkUrl, httpStatus: res.status }); } @@ -83,10 +88,14 @@ export async function ensureCollection(collection, vectorSize, { qdrantUrl, fetc if (!createRes.ok) { throw new UpstreamError(`HTTP ${createRes.status} creating collection`, { surface: checkUrl, httpStatus: createRes.status }); } - return { created: true }; + return { created: true, surface: checkUrl }; } -/** Upsert points into a Qdrant collection. */ +/** + * Upsert points into a Qdrant collection. + * Returns { result, surface } — `surface` is the exact URL fetched (including + * the encoded collection name and ?wait=true); callers report it verbatim. + */ export async function upsertPoints(collection, points, { qdrantUrl, fetchImpl = fetch, timeoutMs = 30000 }) { const url = `${qdrantUrl}/collections/${encodeURIComponent(collection)}/points?wait=true`; let res; @@ -112,15 +121,19 @@ export async function upsertPoints(collection, points, { qdrantUrl, fetchImpl = httpStatus: res.status, }); } - return json; + return { result: json, surface: url }; } -/** Vector search in a Qdrant collection. Returns scored points with payloads. */ +/** + * Vector search in a Qdrant collection. + * Returns { result: scoredPoints[], surface } — `surface` is the exact URL + * fetched (encoded collection name); callers report it verbatim. + */ export async function searchPoints(collection, vector, limit, { qdrantUrl, fetchImpl = fetch, timeoutMs = 30000 }) { const url = `${qdrantUrl}/collections/${encodeURIComponent(collection)}/points/search`; const json = await postJson(url, { vector, limit, with_payload: true }, { fetchImpl, timeoutMs }); if (!Array.isArray(json?.result)) { throw new UpstreamError("Qdrant response missing 'result' array", { surface: url }); } - return json.result; + return { result: json.result, surface: url }; } diff --git a/test/app.test.js b/test/app.test.js index 333da93..ee80ec9 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -17,8 +17,8 @@ const TEST_ENV = { UPSTREAM_TIMEOUT_MS: "500", }; -async function withServer(fetchImpl, fn) { - const config = loadConfig(TEST_ENV); +async function withServer(fetchImpl, fn, envOverrides = {}) { + const config = loadConfig({ ...TEST_ENV, ...envOverrides }); const app = createApp(config, { fetchImpl }); const server = app.listen(0, "127.0.0.1"); await once(server, "listening"); @@ -43,7 +43,7 @@ function fakeFetch(routes) { const impl = async (url, init = {}) => { const method = init.method ?? "GET"; const key = `${method} ${url}`; - calls.push({ key, body: init.body ? JSON.parse(init.body) : undefined }); + calls.push({ key, url: String(url), method, body: init.body ? JSON.parse(init.body) : undefined }); for (const [pattern, handler] of Object.entries(routes)) { if (key === pattern || key.startsWith(pattern)) { return handler({ url, init, body: init.body ? JSON.parse(init.body) : undefined }); @@ -162,6 +162,97 @@ test("/health: embed model missing from a REACHABLE ollama is absent (a true gap }); }); +test("/health: bare EMBED_MODEL matches installed tagged model (nomic-embed-text ~ :latest)", async () => { + // happyRoutes lists nomic-embed-text:latest; config asks for bare name. + await withServer(fakeFetch(happyRoutes()), async (base) => { + const res = await fetch(`${base}/health`); + assert.equal(res.status, 200); + const body = await res.json(); + assert.equal(body.dependencies.embed_model.status, "ok"); + }); +}); + +test("/health: tagged EMBED_MODEL matches installed bare model (reverse direction)", async () => { + const routes = happyRoutes(); + routes["GET http://ollama.test:11436/api/tags"] = () => + jsonResponse({ models: [{ name: "nomic-embed-text" }] }); + await withServer( + fakeFetch(routes), + async (base) => { + const res = await fetch(`${base}/health`); + assert.equal(res.status, 200); + const body = await res.json(); + assert.equal(body.dependencies.embed_model.status, "ok"); + }, + { EMBED_MODEL: "nomic-embed-text:latest" } + ); +}); + +test("/health: when BOTH sides carry tags, tags must match exactly (mismatch => absent)", async () => { + const routes = happyRoutes(); // lists nomic-embed-text:latest + await withServer( + fakeFetch(routes), + async (base) => { + const res = await fetch(`${base}/health`); + assert.equal(res.status, 503); + const body = await res.json(); + assert.equal(body.dependencies.ollama.status, "ok"); + assert.equal(body.dependencies.embed_model.status, "absent"); + }, + { EMBED_MODEL: "nomic-embed-text:v1.5" } + ); +}); + +test("/health: a different model base name never matches (no false ok on shared tag)", async () => { + const routes = happyRoutes(); + routes["GET http://ollama.test:11436/api/tags"] = () => + jsonResponse({ models: [{ name: "mxbai-embed-large:latest" }] }); + await withServer(fakeFetch(routes), async (base) => { + const res = await fetch(`${base}/health`); + const body = await res.json(); + assert.equal(body.dependencies.embed_model.status, "absent"); + }); +}); + +test("/health: 200 tags response WITHOUT a models array is unobservable, never absent", async () => { + for (const wrongShape of [{}, { tags: [{ name: "nomic-embed-text" }] }, { models: "nope" }, null]) { + const routes = happyRoutes(); + routes["GET http://ollama.test:11436/api/tags"] = () => jsonResponse(wrongShape); + await withServer(fakeFetch(routes), async (base) => { + const res = await fetch(`${base}/health`); + assert.equal(res.status, 503); + const body = await res.json(); + assert.equal(body.dependencies.ollama.status, "ok", "reachability verdict stays ok"); + assert.equal( + body.dependencies.embed_model.status, + "unobservable", + `shape ${JSON.stringify(wrongShape)} cannot prove absence` + ); + assert.notEqual(body.dependencies.embed_model.status, "absent"); + assert.equal(body.status, "degraded"); + }); + } +}); + +test("/health: non-JSON 200 tags body is unobservable", async () => { + const routes = happyRoutes(); + routes["GET http://ollama.test:11436/api/tags"] = () => new Response("proxy error", { status: 200 }); + await withServer(fakeFetch(routes), async (base) => { + const res = await fetch(`${base}/health`); + assert.equal(res.status, 503); + const body = await res.json(); + assert.equal(body.dependencies.embed_model.status, "unobservable"); + }); +}); + +test("/health: top-level object carries its own observed_at", async () => { + await withServer(fakeFetch(happyRoutes()), async (base) => { + const res = await fetch(`${base}/health`); + const body = await res.json(); + assert.ok(Date.parse(body.observed_at), "top-level observed_at is a valid timestamp"); + }); +}); + // --------------------------------------------------------------------------- // /v1/embed // --------------------------------------------------------------------------- @@ -271,6 +362,46 @@ test("POST /v1/index with qdrant down returns 502 unobservable (never silently i }); }); +test("POST /v1/index reports a measurement_surface byte-exact with the URL actually fetched", async () => { + const f = fakeFetch(happyRoutes()); + await withServer(f, async (base) => { + const res = await fetch(`${base}/v1/index`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ collection: "notes", items: [{ id: 1, text: "alpha" }] }), + }); + assert.equal(res.status, 200); + const body = await res.json(); + const upsertCall = f.calls.find((c) => c.method === "PUT" && c.url.includes("/points")); + assert.ok(upsertCall, "upsert call was made"); + assert.equal(body.measurement_surface, upsertCall.url, "surface must byte-match the fetched URL"); + assert.equal(body.measurement_surface, "http://qdrant.test:6333/collections/notes/points?wait=true"); + }); +}); + +test("POST /v1/index surface stays byte-exact when the collection name needs URL-encoding", async () => { + const f = fakeFetch({ + ...happyRoutes(), + // NOTE: the /points route must precede the bare collection route (startsWith matching). + "GET http://qdrant.test:6333/collections/my%20notes": () => jsonResponse({ result: {} }, 404), + "PUT http://qdrant.test:6333/collections/my%20notes/points?wait=true": () => + jsonResponse({ result: { operation_id: 7, status: "completed" } }), + "PUT http://qdrant.test:6333/collections/my%20notes": () => jsonResponse({ result: true }), + }); + await withServer(f, async (base) => { + const res = await fetch(`${base}/v1/index`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ collection: "my notes", items: [{ id: 1, text: "alpha" }] }), + }); + assert.equal(res.status, 200); + const body = await res.json(); + const upsertCall = f.calls.find((c) => c.method === "PUT" && c.url.includes("/points")); + assert.equal(body.measurement_surface, upsertCall.url); + assert.equal(body.measurement_surface, "http://qdrant.test:6333/collections/my%20notes/points?wait=true"); + }); +}); + // --------------------------------------------------------------------------- // /v1/search // --------------------------------------------------------------------------- @@ -294,6 +425,58 @@ test("POST /v1/search embeds the query and returns scored payloads", async () => }); }); +test("POST /v1/search reports a measurement_surface byte-exact with the URL actually fetched", async () => { + const f = fakeFetch(happyRoutes()); + await withServer(f, async (base) => { + const res = await fetch(`${base}/v1/search`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ collection: "notes", query: "find alpha" }), + }); + assert.equal(res.status, 200); + const body = await res.json(); + const searchCall = f.calls.find((c) => c.method === "POST" && c.url.includes("/points/search")); + assert.ok(searchCall, "search call was made"); + assert.equal(body.measurement_surface, searchCall.url, "surface must byte-match the fetched URL"); + assert.equal(body.measurement_surface, "http://qdrant.test:6333/collections/notes/points/search"); + }); +}); + +test("POST /v1/search surface stays byte-exact when the collection name needs URL-encoding", async () => { + const f = fakeFetch({ + ...happyRoutes(), + "POST http://qdrant.test:6333/collections/my%20notes/points/search": () => + jsonResponse({ result: [{ id: "a", score: 0.5, payload: { text: "alpha" } }] }), + }); + await withServer(f, async (base) => { + const res = await fetch(`${base}/v1/search`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ collection: "my notes", query: "alpha" }), + }); + assert.equal(res.status, 200); + const body = await res.json(); + const searchCall = f.calls.find((c) => c.method === "POST" && c.url.includes("/points/search")); + assert.equal(body.measurement_surface, searchCall.url); + assert.equal(body.measurement_surface, "http://qdrant.test:6333/collections/my%20notes/points/search"); + }); +}); + +test("POST /v1/embed reports a measurement_surface byte-exact with the URL actually fetched", async () => { + const f = fakeFetch(happyRoutes()); + await withServer(f, async (base) => { + const res = await fetch(`${base}/v1/embed`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ texts: ["hello"] }), + }); + assert.equal(res.status, 200); + const body = await res.json(); + const embedCall = f.calls.find((c) => c.method === "POST" && c.url.includes("/api/embeddings")); + assert.equal(body.measurement_surface, embedCall.url, "surface must byte-match the fetched URL"); + }); +}); + test("POST /v1/search with qdrant down returns 502 unobservable — never an empty result set", async () => { const routes = happyRoutes(); routes["POST http://qdrant.test:6333/collections/notes/points/search"] = () => { From 0b95f2e09a59ef080e073613b35742ac6017346d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 02:37:38 -0400 Subject: [PATCH 2/2] Codex finding: unreadable model entries cannot prove absence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 200 tags response whose models array contains entries without a readable string name is unobservable for the embed model (an unnamed entry could be it) — never absent. Positive matches still count amid malformed siblings; an empty, fully readable list still proves absence. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012T1XF1ZyJL7KW8AVRUJjiD --- src/app.js | 17 ++++++++++++++--- test/app.test.js | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/app.js b/src/app.js index 890142c..11bf3b5 100644 --- a/src/app.js +++ b/src/app.js @@ -84,9 +84,20 @@ export function createApp(config, { fetchImpl = fetch } = {}) { return null; } const found = models.some((m) => modelNamesMatch(m?.name, config.embedModel)); - embedModelResult = probeResult(found ? OK : ABSENT, ollamaSurface, { - model: config.embedModel, - }); + if (found) { + // Positive evidence stands on its own, even amid malformed entries. + embedModelResult = probeResult(OK, ollamaSurface, { model: config.embedModel }); + return null; + } + // Absence is only provable from a FULLY readable list: an entry + // without a readable name could be the model we are looking for. + const allReadable = models.every((m) => typeof m?.name === "string"); + embedModelResult = allReadable + ? probeResult(ABSENT, ollamaSurface, { model: config.embedModel }) + : probeResult(UNOBSERVABLE, ollamaSurface, { + model: config.embedModel, + reason: "models list contains entries without a readable name", + }); return null; // do not override the reachability verdict }, }), diff --git a/test/app.test.js b/test/app.test.js index ee80ec9..6aa7911 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -234,6 +234,47 @@ test("/health: 200 tags response WITHOUT a models array is unobservable, never a } }); +test("/health: models list with unreadable entries cannot prove absence (unobservable)", async () => { + for (const badList of [ + [{}], + [{ model: "nomic-embed-text" }], // wrong field name + [{ name: "qwen3:30b-a3b" }, { name: 42 }], // mixed: one unreadable entry + ]) { + const routes = happyRoutes(); + routes["GET http://ollama.test:11436/api/tags"] = () => jsonResponse({ models: badList }); + await withServer(fakeFetch(routes), async (base) => { + const res = await fetch(`${base}/health`); + const body = await res.json(); + assert.equal( + body.dependencies.embed_model.status, + "unobservable", + `list ${JSON.stringify(badList)} has unreadable entries and cannot prove absence` + ); + }); + } +}); + +test("/health: positive evidence stands even amid malformed sibling entries", async () => { + const routes = happyRoutes(); + routes["GET http://ollama.test:11436/api/tags"] = () => + jsonResponse({ models: [{}, { name: "nomic-embed-text:latest" }] }); + await withServer(fakeFetch(routes), async (base) => { + const res = await fetch(`${base}/health`); + const body = await res.json(); + assert.equal(body.dependencies.embed_model.status, "ok"); + }); +}); + +test("/health: an empty models list IS readable and proves absence", async () => { + const routes = happyRoutes(); + routes["GET http://ollama.test:11436/api/tags"] = () => jsonResponse({ models: [] }); + await withServer(fakeFetch(routes), async (base) => { + const res = await fetch(`${base}/health`); + const body = await res.json(); + assert.equal(body.dependencies.embed_model.status, "absent"); + }); +}); + test("/health: non-JSON 200 tags body is unobservable", async () => { const routes = happyRoutes(); routes["GET http://ollama.test:11436/api/tags"] = () => new Response("proxy error", { status: 200 });