CLAUDE.md

@@ -289,7 +289,14 @@ so failing early is better than an opaque provider 400.
 ### Local Ollama vision fallback
 
 `local_vision:` in `config.yaml` configures a fallback path for image requests
-that find no cloud vision candidate. It ships **disabled** (`enabled: false`).
+that find no cloud vision candidate — the cloud catalog excludes the cost
+leader (deepseek) on vision, so without this fallback every image request
+that would otherwise have routed there 422s instead. It is a **core feature**,
+**enabled by default** (`enabled: true`, both in `config.yaml` and in
+`LocalVisionConfig`'s own default, so a config that omits the section still
+gets it). Disable it explicitly (`enabled: false`) on a host with no local
+Ollama, or one that hasn't pulled the vision model.
+
 When enabled and routing returns no selected model, `_run_local_vision` sends
 the original messages, with `image_url` parts intact, to a local Ollama model
 (`qwen3-vl:4b` by default). The local answer then **replaces** the completion:
@@ -1007,6 +1014,12 @@ match `classifier.model` in `config.yaml`:
 ollama pull mistral-nemo:12b
 ```
 
+`local_vision` ships **enabled** as a core feature (see below), so also pull
+its model unless you're turning it off:
+```bash
+ollama pull qwen3-vl:4b
+```
+
 It does not have to be on this machine. To use one across a VPN, point
 `classifier.base_url` and `verification.base_url` at it and apply
 `deploy/ollama-over-vpn.conf` on the serving host — Ollama binds loopback-only

README.md

@@ -373,7 +373,7 @@ failure — it means the checker had nothing to say, not that the model failed.
 
 ## Weighted Scoring
 
-Four hard filters are applied **before** scoring (not weighted — outright disqualification):
+Six hard filters are applied **before** scoring (not weighted — outright disqualification):
 
 1. `effective_context_window ≥ required_context_tokens`
 2. `tier ≥ required_tier` (from classifier)

capabilities.py

@@ -19,6 +19,7 @@ recorded so dispatcher can observe it, but never fed to a routing filter.
 
 from __future__ import annotations
 
+from collections.abc import Iterator
 from dataclasses import dataclass
 from typing import Any
 
@@ -68,7 +69,18 @@ def detect_capabilities(body: dict[str, Any]) -> RequestCapabilities:
     )
 
 
-def _any_message_has_images(messages: list[Any]) -> bool:
+def iter_image_url_values(messages: list[Any]) -> Iterator[str]:
+    """Every image_url part's URL/data-URI string, across all messages.
+
+    Yields exactly one string per image_url-typed part, even when no URL can
+    be resolved from it (empty string). A part that declares itself an image
+    but carries no readable value must still count toward "this conversation
+    has an image" and still fail the "every part is a safe data: URI" check
+    -- silently skipping it, as an earlier version of this function did,
+    dropped it out of both checks: a degenerate part could then dodge the
+    vision-capability gate AND slip past the SSRF guard that only holds when
+    every yielded value is verified to start with ``data:``.
+    """
     for message in messages:
         if not isinstance(message, dict):
             continue
@@ -76,6 +88,16 @@ def _any_message_has_images(messages: list[Any]) -> bool:
         if not isinstance(content, list):
             continue
         for part in content:
-            if isinstance(part, dict) and part.get("type") == "image_url":
+            if not isinstance(part, dict) or part.get("type") != "image_url":
-                return True
+                continue
-    return False
+            value = part.get("image_url")
+            if isinstance(value, dict):
+                value = value.get("url", "")
+            if not isinstance(value, str):
+                # Bare url key (alternative shape), or nothing resolvable.
+                value = part.get("url") if isinstance(part.get("url"), str) else ""
+            yield value
+
+
+def _any_message_has_images(messages: list[Any]) -> bool:
+    return any(True for _ in iter_image_url_values(messages))

code_plans/shared-helper-unparseable-input-contract.md

@@ -0,0 +1,140 @@
+# Spec + implementation plan: pin down what a shared helper yields on well-typed-but-unparseable input
+
+**Origin.** PR #3 collapsed four independent `messages` → `image_url` part
+traversals into one generator, `capabilities.iter_image_url_values`
+(`code_reviews/capability-gate-followups.md`, item #6). The plan specified
+the two shapes the four originals agreed on (`image_url` as a dict with
+`url`, or as a bare string) but not the case where a part is typed
+`image_url` and carries neither — well-typed, but nothing to extract. The
+merged helper silently skipped that case instead of yielding a placeholder,
+which dropped a degenerate part out of two fail-closed checks at once (see
+`code_reviews/pr-3-capability-gate-followups-review.md` for the live
+repro and the fix). The follow-up opencode session that implemented the fix
+named the general lesson explicitly, which is what this document is
+formalizing: **a plan that proposes collapsing N implementations into one
+shared helper has to specify the helper's contract on every input shape,
+not only the ones the N originals happened to agree on.**
+
+This is a process document, not a bug report — no code changes accompany
+it. It exists so the next plan that proposes a shared helper pins this down
+up front instead of it being rediscovered in review.
+
+---
+
+## Spec
+
+### The problem, generalized
+
+When several independent implementations of "walk this structure and pull
+out X" get unified into one shared helper, each original's handling of its
+*own* edge cases has to be re-derived by the person writing the unification
+— nothing forces them to enumerate it, because the edge case was never the
+reason any of the N implementations were written in the first place. Each
+one just happened to fall into some fallback path. The happy path is easy
+to unify because all N agree on it by construction (they all worked, on
+real traffic, for the common shape). The edge cases are exactly where they
+are most likely to *quietly disagree*, and disagreement there is invisible
+unless a plan makes each implementation's edge-case behavior explicit
+before proposing the replacement.
+
+The specific edge case that bit PR #3 — matches the coarse type tag but the
+payload is missing or empty — deserves its own name because it is a
+distinct category from "wrong type" or "absent": it is a **well-typed but
+unparseable** input. A traversal that filters on `part.get("type") ==
+"image_url"` before doing anything else will always let this shape through
+the filter; what happens next is the part that has to be decided
+deliberately.
+
+### The rule
+
+Any implementation plan that proposes extracting or unifying a shared
+traversal, parser, or extractor helper from multiple existing call sites
+must include an explicit contract: for every input shape the helper will
+see, state what it yields or returns. At minimum, enumerate:
+
+1. **The common/valid shape** — the one all N originals handle the same way.
+2. **Every alternate valid shape** any existing call site already handles
+   (there may be more than one — PR #3 had three: `image_url` as
+   `{"url": ...}`, `image_url` as a bare string, and a bare `url` key on
+   the part itself).
+3. **The well-typed-but-unparseable shape** — passes the coarse type/shape
+   check but the value cannot actually be extracted. State the yielded
+   value explicitly (a sentinel like `""`, not "nothing" / "skip") unless
+   skipping is a deliberate, stated decision.
+4. **The wrong-typed or absent shape** — fails the coarse check. Usually
+   "skip", but say so, so it's a decision and not an assumption.
+
+And critically: **for category 3, list what each of the N originals
+currently does**, not just what the new helper will do. That list is what
+surfaces disagreement before the merge, rather than after. In PR #3, all
+four originals agreed by accident (each one's "nothing extracted" path
+happened to either count the part or fail closed) — but nothing in the
+plan recorded that agreement, so the merge didn't have to preserve it and
+didn't.
+
+### Why "skip silently" is the default failure mode to watch for
+
+A `for`/`if`/`continue` traversal skips by construction unless a branch
+explicitly appends/yields on every path. Converting four such loops into
+one `yield`-per-match generator is a natural, idiomatic simplification —
+and it's exactly the transformation that turns "always contributes
+*something*, even a fail-closed placeholder" into "contributes nothing,
+silently, for a whole category of input." This isn't specific to
+generators — the same risk applies to a shared parser that returns `None`
+for the unparseable case where callers previously each had their own
+`None`-handling that didn't all agree — but generators make it easy to
+introduce, because `continue`-without-yielding reads as the most natural
+thing to write.
+
+---
+
+## Implementation plan
+
+This is a discipline to apply going forward, not a code change. Concretely:
+
+### 1. Template for future plans
+
+When a `code_plans/` (or `code_reviews/`) entry proposes extracting a
+shared helper from multiple call sites, include a table shaped like this
+alongside the proposed code:
+
+| Input shape | Original A | Original B | Original C | ... | New shared helper |
+|---|---|---|---|---|---|
+| common/valid | ... | ... | ... | | ... |
+| alternate valid #1 | ... | ... | ... | | ... |
+| well-typed, unparseable | ... | ... | ... | | ... (state the sentinel) |
+| wrong-typed / absent | skip | skip | skip | | skip |
+
+If any two originals disagree in a row, that disagreement is the thing the
+plan has to resolve on purpose (pick one, document why) rather than let the
+merge resolve by accident.
+
+### 2. No retroactive audit needed right now, but a watch list for next time
+
+Checked the rest of `dispatcher.py` for other traversals of a message's
+multimodal `content` list, since that's the same shape that bit PR #3.
+None of these are currently merged/unified helpers, so none are broken —
+they're independent, single-purpose traversals, not N-collapsed-into-1. But
+if any of them are ever unified (with each other, or with
+`iter_image_url_values`), this spec applies:
+
+- `session_fingerprint` (dispatcher.py:1302) — joins text parts to fingerprint a session.
+- `session_directory` (dispatcher.py:1321) — joins text parts to find a working directory.
+- `estimate_prompt_tokens` (dispatcher.py:1364) — sums text-part lengths for the context estimate.
+- `_last_user_text` (dispatcher.py:1391) — joins the last user turn's text parts.
+
+All four already treat a non-dict or non-string part as "contributes
+nothing" consistently with each other (each filters with
+`isinstance(p, dict)` / `isinstance(p.get("text"), str)` before including
+it), so there's no live disagreement to flag today — this is a watch list,
+not a finding.
+
+### 3. Where this lives
+
+Keep this document as the reference the next relevant plan cites, rather
+than duplicating the rule inline each time. A future plan proposing a
+shared-helper extraction should link back to this file
+(`code_plans/shared-helper-unparseable-input-contract.md`) and fill in the
+contract table from §1 as part of the plan, the same way
+`code_reviews/capability-gate-followups.md` gave concrete before/after code
+for each finding rather than describing it in prose only.

code_reviews/capability-gate-followups.md

@@ -0,0 +1,319 @@
+# Capability-gate follow-ups
+
+Source: `/code-review medium` on `neuralwatt-router-service`, 2026-08-23, run
+against the capability-gate / local-vision-fallback work
+(`4ce9959`..`c4ebc4b`..`cff36f8`). 8 findings came back; 2 are fixed
+(`69d3a8b`: `local_vision.enabled` now ships/defaults `true` as a documented
+core feature, and the `provider/model` prefix is now resolved against the
+catalog for a pin, not only for `auto`). This tracks the remaining 6.
+
+No timeline attached — pick items up in any order except where a dependency
+is called out below. #3 and #6 touch the same two functions and share a
+root cause, so doing them together avoids editing the same lines twice.
+
+---
+
+## 1. Local vision fallback ignores `require_json_mode`
+
+**File:** `dispatcher.py`, fallback trigger at `chat_completions` (~line
+1785-1794); the check function is `_run_local_vision` (~line 1478).
+
+**Problem.** The fallback fires on `caps.has_images and
+cfg.local_vision.enabled` alone:
+
+```python
+if decision.selected is None:
+    if (
+        caps.has_images
+        and cfg.local_vision.enabled
+    ):
+        fallback = _run_local_vision(messages, cfg.local_vision)
+```
+
+It never checks `caps.require_json_mode`. If a request carries both an
+image and `response_format: {type: json_object}`, and JSON mode — not
+vision — is what excluded every candidate, the fallback still fires.
+`_local_vision_response` wraps the local model's free-form text as a normal
+completion with no JSON-mode enforcement, silently breaking the caller's
+`json_object` contract and returning 200 instead of the 422 that would have
+correctly named the missing capability.
+
+**Fix.** Local vision only knows how to answer with prose, so it must not
+be attempted when JSON mode was requested:
+
+```python
+if decision.selected is None:
+    if (
+        caps.has_images
+        and not caps.require_json_mode
+        and cfg.local_vision.enabled
+    ):
+        fallback = _run_local_vision(messages, cfg.local_vision)
+```
+
+When both are true, this now falls through to the existing 422, which
+already lists `caps.require_json_mode` in `limits` (line ~1813-1816) — no
+change needed there.
+
+**Test to add** (`tests/test_chat_completions.py`, alongside
+`test_local_fallback_refuses_a_remote_image_url` and the other
+`local_vision` tests): an image + `response_format: json_object` request,
+with `local_vision.enabled = True` and no cloud vision candidate, must
+still 422 and must not reach the fake local-vision `fake_post`.
+
+---
+
+## 2. Pinned-model capability gate ignores `require_vision`/`require_json_mode` config
+
+**File:** `dispatcher.py`, `_check_pinned_capabilities` (line 1622) and its
+call site (line 1844-1845).
+
+**Problem.** `route()` only applies the vision/json-mode filter when the
+corresponding config flag is on:
+
+```python
+require_vision=cfg.routing.require_vision if req.has_images else False,
+require_json_mode=(
+    cfg.routing.require_json_mode if req.require_json_mode else False
+),
+```
+(`dispatcher.py` ~line 610-613, feeding `routing.rejection_reason`.)
+
+`_check_pinned_capabilities` has no such condition — it checks
+`caps.has_images` / `caps.require_json_mode` directly, with no reference to
+`cfg.routing.require_vision` / `require_json_mode` at all. If an operator
+sets `routing.require_vision: false` (accepting the occasional provider
+400 in exchange for not fail-closing on an unconfirmed flag), `auto`
+traffic honors that, but a pin with an image still gets an unconditional
+422. Same request shape, different outcome, depending only on whether the
+client said `auto` or a real model id.
+
+**Fix.** Covered by the refactor in #6 below — once
+`_check_pinned_capabilities` calls the same shared helper `route()` uses,
+threading the two config flags through is one line each. If #6 is deferred,
+the standalone fix is:
+
+```python
+if cfg.routing.require_vision and caps.has_images and (row is None or row["supports_vision"] != 1):
+    ...
+if cfg.routing.require_json_mode and caps.require_json_mode and (row is None or row["supports_json_mode"] != 1):
+    ...
+```
+
+**Test to add:** with `cfg.routing.require_vision` monkeypatched `False`,
+a pin to a non-vision model with an image must dispatch (200), not 422.
+
+---
+
+## 3. `opencode.json` advertises image support `deepseek-v4-flash` doesn't have
+
+**File:** `opencode.json`, the `deepseek-v4-flash` model entry (line ~39-51).
+
+**Problem.** Confirmed against the live catalog:
+
+```
+$ sqlite3 router.db "SELECT model_id, supports_vision FROM models WHERE model_id='deepseek-v4-flash';"
+deepseek-v4-flash|0
+```
+
+but the opencode entry declares:
+```json
+"deepseek-v4-flash": {
+  "modalities": { "input": ["text", "image"] }
+}
+```
+copied from the other pins, all of which really do support vision
+(`gemma-4-31b`, `kimi-k2.7-code`, `kimi-k3`, `qwen3.6-35b` all read
+`supports_vision = 1`). opencode's UI trusts this and lets a user attach an
+image to the deepseek pin; the request then either 422s (once #2 above is
+fixed) or, today, gets forwarded and fails some other way — either way it's
+a late, confusing failure on a model the client's own config advertised as
+supporting images, instead of the image-attach control being disabled for
+that one pin.
+
+**Fix.** Drop the `modalities` block from the `deepseek-v4-flash` entry (or
+set `"input": ["text"]`). No code change; this is a static config edit.
+
+**Verification.** Since the catalog's vision support can change if
+NeuralWatt updates the model, this is a fact worth re-checking rather than
+assuming — re-run the query above before editing if it's been a while, and
+consider whether `poller.py` should be the source of truth for
+`opencode.json`'s modality lists instead of a hand-maintained copy (out of
+scope here; noting it so it doesn't get re-discovered from scratch next
+time this file needs an update).
+
+---
+
+## 4. README's "Four hard filters" heading undercounts
+
+**File:** `README.md`, line 376.
+
+**Problem.**
+```
+Four hard filters are applied **before** scoring (not weighted — outright disqualification):
+```
+immediately precedes a 6-item list (context, tier, latency/access,
+tool-proficiency, vision, json-mode), and the paragraph right after it
+already says "Filters 4–6" (line 388). Leftover from the vision/json-mode
+filters this diff added without updating the count above them.
+
+**Fix.** One-word change: `Four` → `Six`.
+
+---
+
+## 5. `_check_pinned_capabilities` duplicates the centralized gate logic
+
+**File:** `dispatcher.py` (`_check_pinned_capabilities`, line 1622) vs.
+`routing.py` (`rejection_reason`, line 36, vision/json-mode block at
+line 104-121).
+
+**Problem.** `routing.rejection_reason` already centralizes the fail-closed
+vision/json-mode rule (unknown flag → reject, `False` flag → reject, `True`
+→ pass), used by every routed (`auto`) request. `_check_pinned_capabilities`
+re-implements the same rule with its own raw SQL and a hardcoded pair of
+`if`s for exactly these two flags. Two independent copies of one rule is
+how #2 above happened, and it's how a third gated capability (audio, say)
+would have to be added in `capabilities.py`, `routing.py` (the dataclass
+*and* `rejection_reason`), `TaskRequest`, both `route()` call sites,
+`config.py`/`config.yaml`, *and* a new `if` block here — six-plus places
+for one flag, free to drift.
+
+**Fix.** Extract the capability-flag check out of `rejection_reason` into
+its own function in `routing.py`, since it's the one self-contained piece
+of that rule (unlike context/tier/tool-proficiency, a pin deliberately
+skips those — "a client pinned to one model gets none of the filtering or
+ranking below" is intentional, so this must NOT become "call
+`rejection_reason` for pins too"):
+
+```python
+# routing.py
+def capability_gate_reason(
+    row: dict, *, require_vision: bool = False, require_json_mode: bool = False,
+) -> str | None:
+    """Same fail-closed rule `rejection_reason` uses for vision/json-mode,
+    pulled out so a pinned-model check can reuse it without re-implementing
+    it — see dispatcher._check_pinned_capabilities."""
+    if require_vision:
+        vision = row.get("supports_vision")
+        if vision is None:
+            return "vision(unknown)"
+        if not vision:
+            return "vision(unsupported)"
+    if require_json_mode:
+        jm = row.get("supports_json_mode")
+        if jm is None:
+            return "json_mode(unknown)"
+        if not jm:
+            return "json_mode(unsupported)"
+    return None
+```
+
+`rejection_reason` calls it in place of its inline block (behavior
+unchanged — same checks, same order, same reason strings, so
+`tests/test_routing.py`'s existing assertions on those strings still hold).
+
+`_check_pinned_capabilities` becomes:
+
+```python
+def _check_pinned_capabilities(model_id: str, caps) -> None:
+    conn = _db()
+    try:
+        row = conn.execute(
+            "SELECT supports_vision, supports_json_mode FROM models "
+            "WHERE model_id = ? AND provider = 'neuralwatt'",
+            (model_id,),
+        ).fetchone()
+    finally:
+        conn.close()
+    reason = capability_gate_reason(
+        dict(row) if row else {},
+        require_vision=cfg.routing.require_vision and caps.has_images,
+        require_json_mode=cfg.routing.require_json_mode and caps.require_json_mode,
+    )
+    if reason is not None:
+        capability = "vision" if reason.startswith("vision") else "json mode"
+        raise HTTPException(422, f"{model_id} does not support {capability}...")
+```
+
+This is the same refactor that fixes #2 (config-flag conditioning) — do
+them as one change, not two.
+
+**Test to add:** `tests/test_routing.py` already covers
+`rejection_reason`'s vision/json-mode branches; add a direct unit test for
+`capability_gate_reason` (or just keep testing it through
+`rejection_reason`, since that's now a thin wrapper — either is fine, pick
+whichever the existing test file's style favors). The existing
+`test_a_pinned_non_vision_model_with_an_image_is_a_clear_422` and the two
+prefix-resolution tests added in `69d3a8b` should keep passing unmodified;
+that's the regression check that the refactor didn't change behavior for
+the default (`require_vision: true`) config.
+
+---
+
+## 6. Image-part traversal reimplemented four times
+
+**Files:** `dispatcher.py` — `_count_images` (line 1405),
+`_image_payload_bytes` (line 1424), `_local_vision_data_uris_ok`
+(line 1450); `capabilities.py` — `_any_message_has_images` (line 71).
+
+**Problem.** All four walk the same shape (`messages` → dict with a
+`content` list → parts where `part["type"] == "image_url"`) with slightly
+different unwrapping of the `image_url` value (dict-with-`url`-key vs.
+bare string) between them. A future change to how images are represented
+in the request body has to be applied in four places; missing one silently
+breaks detection, the byte budget, or the SSRF data-URI guard while the
+others keep working. `_run_local_vision` also calls `_count_images` and
+`_image_payload_bytes` twice each — once for the budget check, again to
+log on the failure path — doubling the scan for no reason.
+
+**Fix.** One shared generator that yields each image part once, with the
+`image_url` value already unwrapped to a string:
+
+```python
+# capabilities.py, since dispatcher.py already imports from it
+def iter_image_url_values(messages: list[Any]) -> Iterator[str]:
+    """Every image_url part's URL/data-URI string, across all messages."""
+    for message in messages:
+        if not isinstance(message, dict):
+            continue
+        content = message.get("content")
+        if not isinstance(content, list):
+            continue
+        for part in content:
+            if not isinstance(part, dict) or part.get("type") != "image_url":
+                continue
+            value = part.get("image_url")
+            if isinstance(value, dict):
+                value = value.get("url", "")
+            if isinstance(value, str):
+                yield value
+```
+
+Then:
+- `_any_message_has_images` → `any(True for _ in iter_image_url_values(messages))`
+- `_count_images` → `sum(1 for _ in iter_image_url_values(messages))`
+- `_image_payload_bytes` → `sum(len(v) for v in iter_image_url_values(messages))`
+- `_local_vision_data_uris_ok` → `all(v.startswith("data:") for v in iter_image_url_values(messages))`
+
+And in `_run_local_vision`, compute `_count_images(messages)` and
+`_image_payload_bytes(messages)` once each and reuse the value in both the
+condition and the log call, rather than recomputing for the log line.
+
+**Test to add:** none new — this is a pure refactor behind the same four
+call sites, all of which already have test coverage
+(`test_local_fallback_refuses_a_remote_image_url`,
+`test_the_two_pass_reroute_passes_image_capability`, and the capability
+tests in `tests/test_capabilities.py`). Running the existing suite green is
+the verification.
+
+---
+
+## Suggested order
+
+1. **#1** (json-mode-blind fallback) — standalone, quick, clear correctness fix.
+2. **#5 + #2 together** (extract `capability_gate_reason`, thread config
+   flags through the pinned check) — one refactor fixes both.
+3. **#6** (shared image-part iterator) — touches the same file as #2/#5;
+   convenient to do in the same sitting but not dependent on it.
+4. **#3** (`opencode.json` modality) and **#4** (README count) — trivial,
+   do whenever, in any order.

code_reviews/pr-3-capability-gate-followups-review.md

@@ -0,0 +1,84 @@
+# Review: PR #3, "fix: resolve remaining capability-gate review findings"
+
+**What it was reviewing:** the implementation of
+[`capability-gate-followups.md`](capability-gate-followups.md), done by
+opencode driven through the router itself (`llm-router`) rather than by a
+person. Reviewed by re-reading the diff against the plan, and by running
+the suite locally.
+
+## Verdict: all 6 items done correctly; one gap in a part it added beyond the plan
+
+Each plan item was implemented as specified, and matches the concrete code
+the plan proposed closely enough that it reads as if it were typed from it:
+
+| # | Item | Verified |
+|---|---|---|
+| 1 | Local vision fallback honors `require_json_mode` | Fix is the exact one-line change the plan proposed; test posts an image + `json_object` request and asserts the local fallback's `fake_post` is never called. |
+| 2 | Pinned gate honors `cfg.routing.require_vision`/`require_json_mode` | Threaded through `_check_pinned_capabilities`; test flips `require_vision` off and asserts a non-vision pin now dispatches (200) instead of 422. |
+| 3 | `opencode.json` deepseek-v4-flash modality | `image` dropped from the entry; re-checked live against `router.db` — `supports_vision = 0` for `deepseek-v4-flash`, confirming the edit is correct, not just plan-compliant. |
+| 4 | README "Four hard filters" → "Six" | Matches. |
+| 5 | Extract `capability_gate_reason` | `rejection_reason` now delegates to it; `_check_pinned_capabilities` reuses it via `dict(row)` (correctly worked around `sqlite3.Row` having no `.get()`). 6 new unit tests cover every branch of the extracted function directly. |
+| 6 | Single `iter_image_url_values` generator | All four traversal sites collapsed onto it; `_run_local_vision` now computes count/bytes once each instead of twice. |
+
+Full suite after the PR's own changes: 416 passed (matches the PR
+description's own count).
+
+## The gap: a fail-open regression in `iter_image_url_values`, introduced beyond the plan
+
+The plan's sketch for `iter_image_url_values` only handled the two shapes
+the four original hand-rolled loops handled (`image_url` as a dict with
+`url`, or as a bare string). The merged version added a third branch —
+`elif isinstance(part.get("url"), str))`, a bare `url` key at the part
+level — which is legitimate: it's required to keep a **pre-existing** test
+(`test_image_url_part_sets_has_images`, not touched by this PR) passing,
+since that fixture uses exactly that shape.
+
+But the three-way `if/elif` only yields when one of the three shapes
+resolves. A part that is `{"type": "image_url"}` with **none** of them —
+spec-invalid, but the four original implementations each handled it
+explicitly — now yields nothing at all, and silently drops out of every
+derived check. Confirmed live, before the fix in this pass:
+
+```python
+msgs = [{"role": "user", "content": [{"type": "image_url"}]}]
+# before: has_images=False, data_uris_ok=True   (both wrong)
+# after:  has_images=True,  data_uris_ok=False  (matches pre-PR behavior)
+```
+
+Two consequences, both regressions from the pre-PR code:
+- `_any_message_has_images` → `False`: the request could route to a
+  non-vision model instead of being gated, the exact failure mode
+  `require_vision` fail-closed-on-unknown exists to prevent.
+- `_local_vision_data_uris_ok` → `True`: the SSRF guard (every image part
+  must be a verified `data:` URI before the local model is trusted with it)
+  passed vacuously, since `all()` over zero yielded items is `True`.
+
+No test — old or new — exercised this shape either way, which is why it
+shipped.
+
+## Fix applied in this pass
+
+`capabilities.iter_image_url_values` now always yields exactly one string
+per `image_url`-typed part — `""` when nothing resolves — instead of
+skipping. `""` fails `startswith("data:")` (closes the SSRF gap) and still
+counts toward `any()`/`sum()` (closes the detection gap), matching what the
+four original implementations did before this PR touched them.
+
+Two regression tests added:
+- `tests/test_capabilities.py::test_a_degenerate_image_url_part_still_counts`
+- `tests/test_chat_completions.py::test_local_fallback_refuses_a_degenerate_image_url_part`
+
+Full suite: 418 passed.
+
+## Take for next time
+
+The pattern worth naming: collapsing N independent implementations into one
+shared primitive is exactly where behavior quietly narrows, because each
+original implementation's handling of its own edge cases has to be
+re-derived rather than copied — nothing forces the unifier to enumerate
+what every caller did on the input shapes it doesn't have a test for. Plan
+item #6 anticipated the collapse but not this; a shared traversal helper is
+a good candidate for the plan itself to have specified "what does this
+yield when a part is well-typed but unparseable" up front, rather than
+leaving it to be inferred from four call sites with four different
+fallback behaviors.

config.py

@@ -216,7 +216,7 @@ class VerificationConfig(StrictModel):
 
 
 class LocalVisionConfig(StrictModel):
-    enabled: bool = False
+    enabled: bool = True
     base_url: str = "http://localhost:11434/v1"   # OpenAI-compatible (classifier shape)
     api_key_env: Optional[str] = None
     model: str = "qwen3-vl:4b"

dispatcher.py

@@ -53,11 +53,12 @@ from openai import OpenAI, OpenAIError
 from pydantic import BaseModel, Field
 
 import logs
-from capabilities import detect_capabilities
+from capabilities import detect_capabilities, iter_image_url_values
 from config import RouterConfig, load_config
 from routing import (
     BATCH,
     INTERACTIVE,
+    capability_gate_reason,
     rank_candidates,
     rejection_reason,
     select_candidates,
@@ -1410,15 +1411,7 @@ def _count_images(messages: list[dict]) -> int:
     ``detect_capabilities``. A part counts only when it is a dict whose type
     is ``image_url``, mirroring the detection rule.
     """
-    count = 0
+    return sum(1 for _ in iter_image_url_values(messages))
-    for m in messages:
-        content = m.get("content")
-        if not isinstance(content, list):
-            continue
-        for part in content:
-            if isinstance(part, dict) and part.get("type") == "image_url":
-                count += 1
-    return count
 
 
 def _image_payload_bytes(messages: list[dict]) -> int:
@@ -1430,21 +1423,7 @@ def _image_payload_bytes(messages: list[dict]) -> int:
     image_url value so an oversized upload is skipped rather than POSTed to a
     local model that would reject it.
     """
-    total = 0
+    return sum(len(v) for v in iter_image_url_values(messages))
-    for m in messages:
-        content = m.get("content")
-        if not isinstance(content, list):
-            continue
-        for part in content:
-            if not isinstance(part, dict) or part.get("type") != "image_url":
-                continue
-            parts = part.get("image_url")
-            if isinstance(parts, dict):
-                # Inline base64 lives in `url` as data:image/...;base64,<data>.
-                total += len(parts.get("url", ""))
-            elif isinstance(parts, str):
-                total += len(parts)
-    return total
 
 
 def _local_vision_data_uris_ok(messages: list[dict]) -> bool:
@@ -1460,19 +1439,7 @@ def _local_vision_data_uris_ok(messages: list[dict]) -> bool:
     URL declines the fallback (falls through to the 422) rather than handing a
     network-fetching capability to an unauthenticated caller.
     """
-    for m in messages:
+    return all(v.startswith("data:") for v in iter_image_url_values(messages))
-        content = m.get("content")
-        if not isinstance(content, list):
-            continue
-        for part in content:
-            if not isinstance(part, dict) or part.get("type") != "image_url":
-                continue
-            value = part.get("image_url")
-            if isinstance(value, dict):
-                value = value.get("url", "")
-            if not isinstance(value, str) or not value.startswith("data:"):
-                return False
-    return True
 
 
 def _run_local_vision(messages: list[dict], lv_cfg) -> Optional[str]:
@@ -1493,16 +1460,18 @@ def _run_local_vision(messages: list[dict], lv_cfg) -> Optional[str]:
     if not _local_vision_data_uris_ok(messages):
         logs.warning("local_vision_skip", reason="remote_url")
         return None
-    if _count_images(messages) > lv_cfg.max_images:
+    n_images = _count_images(messages)
+    if n_images > lv_cfg.max_images:
         logs.warning(
             "local_vision_skip", reason="too_many_images",
-            n=_count_images(messages), max=lv_cfg.max_images,
+            n=n_images, max=lv_cfg.max_images,
         )
         return None
-    if _image_payload_bytes(messages) > lv_cfg.max_image_bytes:
+    payload_bytes = _image_payload_bytes(messages)
+    if payload_bytes > lv_cfg.max_image_bytes:
         logs.warning(
             "local_vision_skip", reason="oversized_image",
-            bytes=_image_payload_bytes(messages), max=lv_cfg.max_image_bytes,
+            bytes=payload_bytes, max=lv_cfg.max_image_bytes,
         )
         return None
     url = f"{lv_cfg.base_url.rstrip('/')}/chat/completions"
@@ -1600,13 +1569,32 @@ def _local_vision_response(content: str, *, streaming: bool) -> Response:
     return StreamingResponse(stream(), media_type="text/event-stream")
 
 
+def _model_exists(model_id: str) -> bool:
+    """Whether a bare id names a real routable catalog row.
+
+    Used to decide whether a `provider/model` string a client sent is the
+    client's own provider alias (opencode's `llm-router/...` convention,
+    never a real NeuralWatt id) and should be stripped, or a real id that
+    happens to contain a slash -- which would not resolve once stripped.
+    """
+    conn = _db()
+    try:
+        row = conn.execute(
+            "SELECT 1 FROM models WHERE model_id = ? AND provider = 'neuralwatt'",
+            (model_id,),
+        ).fetchone()
+    finally:
+        conn.close()
+    return row is not None
+
+
 def _check_pinned_capabilities(model_id: str, caps) -> None:
     """Reject a pinned model that cannot satisfy the request's capabilities.
 
     Reads the ``models`` row by ``model_id`` and fails closed on any missing
-    or NULL capability flag, matching the routing gate: a capability that
+    or NULL capability flag, matching the routing gate. Honors the
-    cannot be confirmed is treated as absent, because dispatching to a model
+    ``cfg.routing.require_*`` config flags so a pin and routed traffic face
-    that might lack it is a guaranteed provider 400.
+    the same gate — see issue #2 in the capability-gate follow-ups doc.
     """
     conn = _db()
     try:
@@ -1617,15 +1605,17 @@ def _check_pinned_capabilities(model_id: str, caps) -> None:
         ).fetchone()
     finally:
         conn.close()
-    if caps.has_images and (row is None or row["supports_vision"] != 1):
+    reason = capability_gate_reason(
-        raise HTTPException(
+        dict(row) if row else {},
-            422,
+        require_vision=cfg.routing.require_vision and caps.has_images,
-            f"{model_id} does not support vision; the request carries image parts",
+        require_json_mode=cfg.routing.require_json_mode and caps.require_json_mode,
     )
-    if caps.require_json_mode and (row is None or row["supports_json_mode"] != 1):
+    if reason is not None:
+        capability = "vision" if reason.startswith("vision") else "json mode"
         raise HTTPException(
             422,
-            f"{model_id} does not support json mode; the response_format requires it",
+            f"{model_id} does not support {capability}; "
+            f"the request {'carries image parts' if 'vision' in reason else 'requires response_format'}",
         )
 
 
@@ -1703,12 +1693,20 @@ def chat_completions(body: dict[str, Any], background: BackgroundTasks):
     caps = detect_capabilities(body)
 
     requested = body.get("model") or ROUTER_MODEL
-    # Some clients (opencode among them) send the model as `provider/model`.
+    # Some clients (opencode among them) send the model as `provider/model`,
-    # Real ids can contain slashes too (`deepseek-ai/DeepSeek-V4-Flash`), so
+    # for a pin as much as for `auto`. Real ids can contain slashes too
-    # only the virtual names are matched against a stripped suffix.
+    # (`deepseek-ai/DeepSeek-V4-Flash`), so a stripped suffix is only trusted
+    # once it resolves to a real catalog row -- otherwise the id, slash and
+    # all, is what gets dispatched, on the chance the slash is really part of
+    # it. Without this, a pin sent as `llm-router/gemma-4-31b` kept its
+    # prefix past this point: the capability pre-check looked up a model_id
+    # the catalog has never heard of, found no row, and fail-closed a pin
+    # that could satisfy the request into a false 422 -- and even past that
+    # check, the same unstripped id would have gone upstream and drawn a 400
+    # from NeuralWatt, which only knows the bare form.
     bare = requested.rsplit("/", 1)[-1]
     wants_routing = bare in (ROUTER_MODEL, ROUTER_MODEL_BATCH)
-    if wants_routing:
+    if wants_routing or (bare != requested and _model_exists(bare)):
         requested = bare
 
     logs.debug(
@@ -1758,6 +1756,7 @@ def chat_completions(body: dict[str, Any], background: BackgroundTasks):
         if decision.selected is None:
             if (
                 caps.has_images
+                and not caps.require_json_mode
                 and cfg.local_vision.enabled
             ):
                 fallback = _run_local_vision(messages, cfg.local_vision)

opencode.json

@@ -44,8 +44,7 @@
           },
           "modalities": {
             "input": [
-              "text",
+              "text"
-              "image"
             ]
           }
         },

routing.py

@@ -33,6 +33,32 @@ INTERACTIVE = "interactive"
 BATCH = "batch"
 
 
+def capability_gate_reason(
+    row: dict, *, require_vision: bool = False, require_json_mode: bool = False,
+) -> str | None:
+    """Same fail-closed rule ``rejection_reason`` uses for vision/json-mode,
+    pulled out so a pinned-model check can reuse it without re-implementing
+    it — see dispatcher._check_pinned_capabilities.
+
+    Returns a reason string when the gate fails, or None when the row passes.
+    Reason order: vision(unknown), vision(unsupported), json_mode(unknown),
+    json_mode(unsupported).
+    """
+    if require_vision:
+        vision = row.get("supports_vision")
+        if vision is None:
+            return "vision(unknown)"
+        if not vision:
+            return "vision(unsupported)"
+    if require_json_mode:
+        jm = row.get("supports_json_mode")
+        if jm is None:
+            return "json_mode(unknown)"
+        if not jm:
+            return "json_mode(unsupported)"
+    return None
+
+
 def rejection_reason(
     row: dict,
     *,
@@ -107,18 +133,11 @@ def rejection_reason(
     # absent means "cannot confirm the capability", and routing a request that
     # needs vision or JSON mode to a model that might lack it is a guaranteed
     # provider 400. Same precedent as the context(unknown) gate above.
-    if require_vision:
+    gate = capability_gate_reason(
-        vision = row.get("supports_vision")
+        row, require_vision=require_vision, require_json_mode=require_json_mode,
-        if vision is None:
+    )
-            return "vision(unknown)"
+    if gate is not None:
-        if not vision:
+        return gate
-            return "vision(unsupported)"
-    if require_json_mode:
-        jm = row.get("supports_json_mode")
-        if jm is None:
-            return "json_mode(unknown)"
-        if not jm:
-            return "json_mode(unsupported)"
 
     if min_tool_proficiency is not None:
         tool_score = row.get("tool_proficiency")

tests/test_capabilities.py

@@ -4,7 +4,7 @@ Each case proves one detection fires and that the others stay False, mirroring
 the "rejects and does not over-reject" shape of test_routing.py.
 """
 
-from capabilities import detect_capabilities
+from capabilities import detect_capabilities, iter_image_url_values
 
 
 def test_no_parts_yields_no_capabilities():
@@ -25,6 +25,15 @@ def test_image_url_part_sets_has_images():
     assert detect_capabilities(body).has_images is True
 
 
+def test_a_degenerate_image_url_part_still_counts():
+    # No `image_url` key and no bare `url` key -- spec-invalid, but a part
+    # that declares itself an image must still count as one rather than
+    # silently vanishing because its value can't be resolved. Skipping it
+    # would let a malformed part dodge the vision-capability gate.
+    body = {"messages": [{"content": [{"type": "image_url"}]}]}
+    assert detect_capabilities(body).has_images is True
+
+
 def test_image_in_an_earlier_message_counts():
     body = {
         "messages": [
@@ -82,3 +91,78 @@ def test_reasoning_effort_sets_has_reasoning_request():
 
 def test_reasoning_param_sets_has_reasoning_request():
     assert detect_capabilities({"reasoning": {"enabled": True}}).has_reasoning_request is True
+
+
+# --- iter_image_url_values contract --------------------------------------
+#
+# Pins the shared helper's contract across the four input shapes of
+# code_plans/shared-helper-unparseable-input-contract.md. The point of this
+# test is the same as the contract: the well-typed-but-unparseable shape
+# must YIELD ITS SENTINEL ("") rather than be skipped silently, because
+# skipping drops the part out of the fail-closed image checks at once.
+
+
+def test_iter_image_url_values_yields_valid_dict_url():
+    # Common/valid shape: image_url as a dict carrying `url`.
+    messages = [{"content": [{"type": "image_url", "image_url": {"url": "https://x/img.png"}}]}]
+    assert list(iter_image_url_values(messages)) == ["https://x/img.png"]
+
+
+def test_iter_image_url_values_alternate_valid_shapes():
+    # Alternate valid shapes the four originals agreed on:
+    # image_url as a bare string, and a bare `url` key on the part itself.
+    messages = [
+        {"content": [{"type": "image_url", "image_url": "https://x/bare.png"}]},
+        {"content": [{"type": "image_url", "url": "https://x/barekey.png"}]},
+    ]
+    assert list(iter_image_url_values(messages)) == [
+        "https://x/bare.png",
+        "https://x/barekey.png",
+    ]
+
+
+def test_iter_image_url_values_well_typed_unparseable_yields_sentinel():
+    # Well-typed but unparseable: the part declares itself an image_url but
+    # carries neither `image_url` nor a bare `url` that resolves. It must
+    # still yield SOMETHING (the "" sentinel) rather than vanish, so a
+    # degenerate part cannot slip out of the fail-closed checks. This is the
+    # shape the plan exists to pin down.
+    messages = [{"content": [{"type": "image_url"}]}]
+    assert list(iter_image_url_values(messages)) == [""]
+
+
+def test_iter_image_url_values_wrong_payload_still_yields_sentinel():
+    # The `image_url` key is present but its value is not a dict or a string
+    # (an int, say). Presence of the key is enough to claim "this is an
+    # image"; the payload fails to resolve, so it yields the "" sentinel
+    # rather than being skipped -- the same fail-closed contract as a part
+    # with no key at all.
+    messages = [{"content": [{"type": "image_url", "image_url": 42}]}]
+    assert list(iter_image_url_values(messages)) == [""]
+
+
+def test_iter_image_url_values_skips_wrong_typed_and_absent():
+    # Wrong-typed / absent shapes fail the coarse check and are skipped;
+    # they contribute nothing and yield nothing.
+    messages = [
+        "not a dict",
+        {"content": "plain string, not a list"},
+        {"content": [{"type": "text", "text": "hello"}]},
+        {"content": [{"type": "tool_use", "id": "t1"}]},
+    ]
+    assert list(iter_image_url_values(messages)) == []
+
+
+def test_iter_image_url_values_mixed_shapes_preserve_order():
+    # A degenerate part in the middle must still count (as "") and not
+    # disturb the ordering of the parts around it.
+    messages = [
+        {"content": [{"type": "image_url", "url": "https://x/1.png"}]},
+        {"content": [{"type": "image_url"}]},
+        {"content": [{"type": "image_url", "image_url": {"url": "https://x/2.png"}}]},
+    ]
+    assert list(iter_image_url_values(messages)) == [
+        "https://x/1.png",
+        "",
+        "https://x/2.png",
+    ]

tests/test_chat_completions.py

@@ -565,6 +565,34 @@ def test_no_vision_model_uses_local_fallback_when_enabled(router, monkeypatch):
     ), "the image_url part must survive into the local call"
 
 
+def test_local_fallback_skips_when_json_mode_is_requested(router, monkeypatch):
+    """Local vision answers in prose; a json_object request must not use it.
+
+    JSON mode -- not vision -- is what excluded every cloud candidate here, so
+    the fallback must not fire or it would silently break the json_object
+    contract with free-form text instead of the 422 naming the missing
+    capability.
+    """
+    client, calls, db_path = router
+    _drop_cheap_by_tier(dispatcher, db_path)
+    monkeypatch.setattr(dispatcher.cfg.local_vision, "enabled", True)
+    _local_vision_fake(monkeypatch, calls, content="should not be used")
+
+    resp = client.post(
+        "/v1/chat/completions",
+        json={
+            "model": "auto",
+            "messages": _image_messages(),
+            "response_format": {"type": "json_object"},
+        },
+    )
+
+    assert resp.status_code == 422
+    assert "json" in resp.json()["detail"]
+    local = [c for c in calls if c.get("local")]
+    assert not local, "the local vision fallback must not run for a json_object request"
+
+
 def test_local_fallback_respects_streaming(router, monkeypatch):
     client, calls, db_path = router
     _drop_cheap_by_tier(dispatcher, db_path)
@@ -660,6 +688,59 @@ def test_a_pinned_non_vision_model_with_an_image_is_a_clear_422(router):
     assert not calls, "no provider call should happen for an impossible pin"
 
 
+def test_a_pinned_non_vision_model_dispatches_when_vision_gate_is_off(router, monkeypatch):
+    """The pin check honors `routing.require_vision`, like routed traffic.
+
+    `require_vision: false` accepts the occasional provider 400 in exchange
+    for not fail-closing on an unconfirmed flag; a pinned model must face the
+    same config switch as `auto` routing, not an unconditional 422.
+    """
+    client, calls, _ = router
+    monkeypatch.setattr(dispatcher.cfg.routing, "require_vision", False)
+
+    resp = client.post(
+        "/v1/chat/completions",
+        json={"model": DEAR, "messages": _image_messages()},
+    )
+
+    assert resp.status_code == 200
+    assert calls[0]["body"]["model"] == DEAR, "the pin must dispatch as asked"
+
+
+def test_a_provider_prefixed_pin_is_resolved_for_the_capability_check(router):
+    """opencode sends a pin as `provider/model` too, not just for `auto`.
+
+    Before the fix, `requested` kept its `llm-router/` prefix past this
+    point: the capability pre-check looked up a model_id the catalog has
+    never heard of, found no row, and fail-closed a genuinely capable pin
+    into a false 422. It must resolve to the bare id and go through, and the
+    provider must see the bare id too -- NeuralWatt has never heard of
+    `llm-router/...` either.
+    """
+    client, calls, _ = router
+    resp = client.post(
+        "/v1/chat/completions",
+        json={"model": f"llm-router/{CHEAP}", "messages": _image_messages()},
+    )
+
+    assert resp.status_code == 200
+    assert calls[0]["body"]["model"] == CHEAP
+    assert resp.headers["X-Router-Model"] == CHEAP
+
+
+def test_a_provider_prefixed_pin_still_fails_closed_when_incapable(router):
+    """The prefix-resolution fix must not turn into a bypass."""
+    client, calls, _ = router
+    resp = client.post(
+        "/v1/chat/completions",
+        json={"model": f"llm-router/{DEAR}", "messages": _image_messages()},
+    )
+
+    assert resp.status_code == 422
+    assert "vision" in resp.json()["detail"]
+    assert not calls, "no provider call should happen for an impossible pin"
+
+
 def test_the_two_pass_reroute_passes_image_capability(router):
     """The measured-context reroute must carry the image gate, not lose it."""
     client, calls, _ = router
@@ -706,6 +787,36 @@ def test_local_fallback_refuses_a_remote_image_url(router, monkeypatch):
     assert not local, "a remote image URL must never reach the local vision endpoint"
 
 
+def test_local_fallback_refuses_a_degenerate_image_url_part(router, monkeypatch):
+    """A part that declares itself an image but carries no readable value at
+    all must still fail the "every part is a verified data: URI" check --
+    silently skipping it would let it slip past the SSRF guard the same way
+    a remote URL is refused above."""
+    client, calls, db_path = router
+    _drop_cheap_by_tier(dispatcher, db_path)
+    monkeypatch.setattr(dispatcher.cfg.local_vision, "enabled", True)
+    _local_vision_fake(monkeypatch, calls, content="should not be used")
+
+    messages = [
+        {
+            "role": "user",
+            "content": [
+                {"type": "text", "text": "describe this"},
+                {"type": "image_url"},
+            ],
+        }
+    ]
+    resp = client.post(
+        "/v1/chat/completions",
+        json={"model": "auto", "messages": messages},
+    )
+
+    assert resp.status_code == 422
+    assert "vision" in resp.json()["detail"]
+    local = [c for c in calls if c.get("local")]
+    assert not local, "an unverifiable image part must never reach the local vision endpoint"
+
+
 def test_local_fallback_wont_masquerade_an_empty_answer_as_200(router, monkeypatch):
     """A 200 with empty content is a failed answer, and must fall through to 422
     rather than return an empty 200 — a hidden failure must not look like a win."""

tests/test_config_endpoints.py

@@ -203,7 +203,7 @@ def test_the_shipped_config_loads_with_the_new_keys(raw):
     assert loaded.routing.require_vision is True
     assert loaded.routing.require_json_mode is True
     assert loaded.local_vision.model == "qwen3-vl:4b"
-    assert loaded.local_vision.enabled is False
+    assert loaded.local_vision.enabled is True
 
 
 def test_a_typo_in_require_vision_is_rejected(raw):
@@ -222,7 +222,7 @@ def test_local_vision_can_be_disabled_from_config(raw):
 def test_local_vision_defaults_when_absent(raw):
     cfg = copy.deepcopy(raw)
     cfg.pop("local_vision")
-    assert RouterConfig(**cfg).local_vision.enabled is False
+    assert RouterConfig(**cfg).local_vision.enabled is True
 
 
 def test_nonpositive_local_timeout_is_rejected(raw):

tests/test_routing.py

@@ -7,6 +7,7 @@ proving it rejects and a case proving it does not over-reject.
 import pytest
 
 from routing import (
+    capability_gate_reason,
     is_eligible,
     rank_candidates,
     rejection_reason,
@@ -440,6 +441,55 @@ def test_reasons_are_single_tokens():
         assert " " not in reason
 
 
+# --- capability_gate_reason (the extracted flag rule) -----------------------
+#
+# rejection_reason delegates its vision/json-mode arm to this function, and
+# dispatcher._check_pinned_capabilities reuses it so a pinned-model check and
+# routed traffic cannot drift apart. It is unit-tested directly because the
+# pinned-model path has no other cheap way to exercise these branches without
+# a full request round-trip.
+
+def test_capability_gate_passes_when_not_required():
+    # With no capability required, a model that lacks both still passes.
+    assert capability_gate_reason(_row(supports_vision=0, supports_json_mode=0)) is None
+
+
+def test_capability_gate_rejects_missing_vision_flag():
+    assert capability_gate_reason(
+        _row(supports_vision=None), require_vision=True
+    ) == "vision(unknown)"
+
+
+def test_capability_gate_rejects_vision_unsupported():
+    assert capability_gate_reason(
+        _row(supports_vision=0), require_vision=True
+    ) == "vision(unsupported)"
+
+
+def test_capability_gate_rejects_missing_json_mode_flag():
+    assert capability_gate_reason(
+        _row(supports_json_mode=None), require_json_mode=True
+    ) == "json_mode(unknown)"
+
+
+def test_capability_gate_rejects_json_mode_unsupported():
+    assert capability_gate_reason(
+        _row(supports_json_mode=0), require_json_mode=True
+    ) == "json_mode(unsupported)"
+
+
+def test_capability_gate_admits_a_row_that_supports_both():
+    assert capability_gate_reason(
+        _row(supports_vision=1, supports_json_mode=1),
+        require_vision=True, require_json_mode=True,
+    ) is None
+
+
+def test_capability_gate_unknown_reason_on_empty_row():
+    assert capability_gate_reason({}, require_vision=True) == "vision(unknown)"
+    assert capability_gate_reason({}, require_json_mode=True) == "json_mode(unknown)"
+
+
 def test_is_eligible_still_agrees_with_the_reason():
     """One copy of the rules, two views of it."""
     for row in (_row(), _row(tier=1), _row(latency_class="flex")):