6krrt/code_reviews/capability-gate-followups.md

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:

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:

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:

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:

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:

"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 ifs 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"):

# 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:

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:

# 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.