Fix review findings: exact measurement surfaces, symmetric model matching, wrong-shape tags = unobservable

- 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 <[email protected]> Claude-Session: https://claude.ai/code/session_012T1XF1ZyJL7KW8AVRUJjiD
2026-08-22 02:35:23 -04:00
parent c4fb46a24a
commit 711f32e86e
3 changed files with 261 additions and 27 deletions
@@ -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("<html>proxy error</html>", { 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"] = () => {