fix(catalog): ingest llama.cpp multimodal and dual-envelope metadata

src/codex/catalog/provider-fetch.ts

@@ -990,6 +990,11 @@ function modelInputModalities(
   if (capabilityRecord?.vision === false) return ["text"];
   if (capabilityRecord?.vision === true || capabilities?.some(value => (
     value === "vision" || value === "image-input" || value === "image_input"
+    // llama.cpp and Ollama-compatible servers report vision as "multimodal" —
+    // it is the only image signal those servers emit (#1797). Mapped to the
+    // closed `text|image` enum rather than passed through: an out-of-enum
+    // modality makes Codex reject the entire catalog file.
+    || value === "multimodal"
   ))) {
     return ["text", "image"];
   }

src/providers/model-discovery.ts

@@ -331,12 +331,83 @@ export function extractModelEnvelopeRows(
 }
 
 /** Validate, bound, deduplicate, and declaratively filter OpenAI `{data:[...]}` or top-level arrays (Together `#617`). */
+/**
+ * Metadata a sibling `models[]` array may contribute to an ALREADY-ADMITTED
+ * `data[]` row (#1797).
+ *
+ * llama.cpp serves a dual-envelope body: an Ollama-style `models[]` array
+ * carrying `capabilities` alongside the OpenAI-style `data[]` array carrying
+ * `meta`, so the two halves of one model's metadata never meet and a server
+ * that truthfully advertises "multimodal" produced an image-blind row.
+ *
+ * Two boundaries make this safe, and both were added after review found the
+ * first attempt unsound:
+ *
+ * 1. It runs AFTER admission filtering. Enriching first let a sibling supply
+ *    the exact field a provider filter requires — reproduced against the real
+ *    Chutes policy, where a row lacking `supported_features: ["tools"]` was
+ *    admitted once a same-id sibling provided it. Enrichment may change what is
+ *    KNOWN about a model, never WHICH models are published.
+ * 2. Only the capability keys #1797 needs are copied. A blanket "fill every
+ *    absent key" made the untrusted `models[]` array a way into any field the
+ *    pipeline consumes.
+ *
+ *    The list deliberately EXCLUDES `supported_features` and `features`, even
+ *    though both are capability-shaped: they are the two keys real provider
+ *    filters test (`registry.ts:1591` requires `supported_features` to contain
+ *    "tools"; `registry.ts:1883` tests `features.tool_use`). Ordering already
+ *    prevents a sibling from flipping an admission verdict, but a key that is
+ *    both enrichable and filter-relevant is one refactor away from becoming a
+ *    bypass again. #1797 does not need them.
+ */
+const SIBLING_ENRICHABLE_KEYS = new Set(["capabilities", "modalities", "input_modalities"]);
+
+type SiblingIndex = Map<string, Record<string, unknown> | null>;
+
+function buildSiblingIndex(value: unknown, limit: number): SiblingIndex | null {
+  const record = plainObject(value);
+  const sibling = record?.models;
+  if (!Array.isArray(sibling) || sibling.length === 0 || sibling.length > limit) return null;
+
+  const byId: SiblingIndex = new Map();
+  for (const raw of sibling) {
+    const entry = plainObject(raw);
+    if (!entry) continue;
+    for (const key of ["id", "model", "name"]) {
+      const id = entry[key];
+      if (typeof id !== "string" || id.length === 0) continue;
+      // `null` marks an ambiguous id: two sibling entries claim it, so neither
+      // can be attributed with confidence and both are ignored.
+      byId.set(id, byId.has(id) && byId.get(id) !== entry ? null : entry);
+    }
+  }
+  return byId.size > 0 ? byId : null;
+}
+
+function enrichAdmittedModel(item: ProviderModelsApiItem, siblings: SiblingIndex): ProviderModelsApiItem {
+  const extra = siblings.get(item.id);
+  if (!extra) return item;
+  let merged: Record<string, unknown> | null = null;
+  for (const key of SIBLING_ENRICHABLE_KEYS) {
+    if (!(key in extra) || key in item) continue;
+    merged ??= { ...(item as Record<string, unknown>) };
+    merged[key] = extra[key];
+  }
+  return (merged ?? item) as ProviderModelsApiItem;
+}
+
+function plainObject(value: unknown): Record<string, unknown> | null {
+  return typeof value === "object" && value !== null && !Array.isArray(value)
+    ? value as Record<string, unknown>
+    : null;
+}
 export function extractProviderModelItems(
   value: unknown,
   discovery: ResolvedProviderModelDiscovery,
 ): ProviderModelItemsResult {
   const limit = positiveIntegerAtMost(discovery.maxModels, MODEL_DISCOVERY_MAX_MODELS);
   let data: unknown[];
+  let siblings: SiblingIndex | null = null;
   if (Array.isArray(value)) {
     // Together-style top-level /models arrays. Catalog discovery must not treat a stray
     // `models` key on openai-chat responses as valid — only `data` envelopes or top-level arrays.
@@ -346,6 +417,7 @@ export function extractProviderModelItems(
     const envelope = extractModelEnvelopeRows(value, discovery.maxModels, ["data"]);
     if (!envelope.ok) return envelope;
     data = envelope.rows;
+    siblings = buildSiblingIndex(value, limit);
   }
 
   const items: ProviderModelsApiItem[] = [];
@@ -363,9 +435,15 @@ export function extractProviderModelItems(
       if (!isValidModelDiscoveryModelId(finalId)) continue;
     }
     const item = finalId === id ? raw as ProviderModelsApiItem : { ...(raw as ProviderModelsApiItem), id: finalId };
+    // Admission is decided on the ORIGINAL `data[]` row, before any sibling
+    // enrichment. Merging first let a `models[]` entry supply the very field a
+    // provider filter requires — reproduced against the real Chutes policy,
+    // where a row lacking `supported_features: ["tools"]` was admitted once a
+    // same-id sibling provided it. Enrichment must never change WHICH models
+    // are published, only what is known about an already-admitted one.
     if (!providerModelMatchesDiscoveryFilter(item, discovery.spec?.filter) || seen.has(finalId)) continue;
     seen.add(finalId);
-    items.push(item);
+    items.push(siblings ? enrichAdmittedModel(item, siblings) : item);
   }
   return { ok: true, items, rawCount: data.length };
 }

tests/catalog-llamacpp-capabilities.test.ts

@@ -3,16 +3,17 @@ import { catalogHintsFromModelsApiItem } from "../src/codex/catalog/provider-fet
 import { extractProviderModelItems } from "../src/providers/model-discovery";
 
 /**
- * Regression coverage for the context half of #1797.
+ * Regression coverage for #1797 (llama.cpp dual-envelope metadata).
  *
- * A llama.cpp server reports its served context under `meta.n_ctx`, which was in
- * none of the recognized context fields, so a correct local server produced no
- * context evidence at all.
+ * A llama.cpp server splits one model across two arrays: an Ollama-style
+ * `models[]` carrying `capabilities` and an OpenAI-style `data[]` carrying
+ * `meta`. Discovery reads only `data[]`, so the served context (`meta.n_ctx`)
+ * and the image signal (`multimodal`) never met and a correct server produced
+ * a context-unknown, image-blind row.
  *
- * The image half of #1797 is NOT fixed here and is characterized below: the
- * `multimodal` token lives in the Ollama-style `models[]` array while discovery
- * deliberately reads only `data[]`, and even a merged item would stay
- * image-unknown because `multimodal` is not a recognized capability string.
+ * Both halves now resolve. The tests below also pin the boundary that makes
+ * the join safe: admission is decided on the original `data[]` row before any
+ * enrichment, and only capability keys are copied.
  */
 
 const VERBATIM_LLAMACPP_BODY = {
@@ -62,9 +63,10 @@ describe("llama.cpp served context ingestion (#1797)", () => {
     expect(hints.contextWindow).toBe(32768);
   });
 
-  test("the dual-envelope body yields context but still no image evidence", () => {
-    // Characterization of the KNOWN remaining gap in #1797, so the follow-up fix
-    // has a live witness and a test to flip rather than a prose claim.
+  test("the dual-envelope body now yields BOTH context and image evidence", () => {
+    // Was a characterization of the #1797 gap: the multimodal token lived in
+    // models[] while discovery read only data[], so the row stayed image-blind.
+    // Both halves are now joined on exact id, and multimodal maps to image.
     const extracted = extractProviderModelItems(VERBATIM_LLAMACPP_BODY, {
       maxModels: 100,
     } as never);
@@ -74,7 +76,67 @@ describe("llama.cpp served context ingestion (#1797)", () => {
 
     const hints = catalogHintsFromModelsApiItem("lidge", items[0] as never);
     expect(hints.contextWindow).toBe(262144);
-    // The "multimodal" token was discarded with models[]; unknown, never false.
-    expect(hints.inputModalities).toBeUndefined();
+    expect(hints.inputModalities).toEqual(["text", "image"]);
+  });
+
+  test("a data[] value is never overridden by its sibling", () => {
+    // models[] is exactly the key discovery refuses to trust as a source of
+    // models. Enrichment fills only ABSENT keys, so an authoritative data[]
+    // entry always wins.
+    const extracted = extractProviderModelItems({
+      models: [{ id: "m", context_length: 999 }],
+      data: [{ id: "m", context_length: 111 }],
+    }, { maxModels: 100 } as never);
+    const items = (extracted as { ok: true; items: Array<Record<string, unknown>> }).items;
+    expect(items[0]!.context_length).toBe(111);
+  });
+
+  test("a model present only in the sibling array is still ignored", () => {
+    // Membership is decided entirely by data[]; the conservative boundary that
+    // refuses a stray models key is preserved.
+    const extracted = extractProviderModelItems({
+      models: [{ id: "ghost" }],
+      data: [{ id: "real" }],
+    }, { maxModels: 100 } as never);
+    const items = (extracted as { ok: true; items: Array<Record<string, unknown>> }).items;
+    expect(items.map(i => i.id)).toEqual(["real"]);
+  });
+
+  test("an ambiguous sibling id is skipped rather than guessed", () => {
+    const extracted = extractProviderModelItems({
+      models: [{ id: "m", context_length: 999 }, { id: "m", context_length: 555 }],
+      data: [{ id: "m" }],
+    }, { maxModels: 100 } as never);
+    const items = (extracted as { ok: true; items: Array<Record<string, unknown>> }).items;
+    expect(items[0]!.context_length).toBeUndefined();
+  });
+  test("sibling metadata cannot admit a model the provider filter rejects", () => {
+    // Enrichment used to run BEFORE admission filtering, so a models[] entry
+    // could supply the exact field a filter required. Reproduced against the
+    // real Chutes policy: a row without supported_features:["tools"] was
+    // admitted once a same-id sibling provided it. Enrichment may change what
+    // is KNOWN about a model, never WHICH models are published.
+    const extracted = extractProviderModelItems({
+      models: [{ id: "not-proven-tool-capable", supported_features: ["tools"] }],
+      data: [{ id: "not-proven-tool-capable" }],
+    }, {
+      maxModels: 100,
+      spec: { filter: { allOf: [{ path: ["supported_features"], containsAny: ["tools"] }] } },
+    } as never);
+
+    const items = (extracted as { ok: true; items: Array<Record<string, unknown>> }).items;
+    expect(items).toEqual([]);
+  });
+
+  test("only capability keys are enriched, not arbitrary fields", () => {
+    // A blanket fill-every-absent-key made the untrusted models[] array a way
+    // into any field the pipeline consumes.
+    const extracted = extractProviderModelItems({
+      models: [{ id: "m", context_length: 999, owned_by: "hostile" }],
+      data: [{ id: "m" }],
+    }, { maxModels: 100 } as never);
+
+    const items = (extracted as { ok: true; items: Array<Record<string, unknown>> }).items;
+    expect(items[0]).toEqual({ id: "m" });
   });
 });