fix: resolve remaining capability-gate review findings #3

Merged
alee merged 6 commits from neuralwatt-router-service into main 2026-08-23 20:02:41 +00:00
Owner

Implements the bugs_to_fix/capability-gate-followups.md plan (#1-#6).

Changes

  • #1 - Local vision fallback now honors require_json_mode: an image + json_object request falls through to the 422 naming the missing capability instead of returning prose that breaks the JSON contract.
  • #2 - The pinned-model capability check now honors cfg.routing.require_vision / require_json_mode, so a pin and routed traffic face the same gate.
  • #3 - Drop 'image' modality from deepseek-v4-flash in opencode.json (supports_vision = 0), verified against the live catalog.
  • #4 - README hard-filter count Four -> Six.
  • #5 - Extract routing.capability_gate_reason(), shared by rejection_reason and the pinned check, so the flag rule cannot drift between the two.
  • #6 - Single iter_image_url_values() generator replaces four image-part traversals; _run_local_vision computes count/bytes once.

Tests

Adds 9 tests (capability_gate_reason unit coverage, json-mode-gated local fallback, vision-gate-off for pins). Full suite: 416 passing (was 407).

Implements the bugs_to_fix/capability-gate-followups.md plan (#1-#6). ## Changes - **#1** - Local vision fallback now honors `require_json_mode`: an image + json_object request falls through to the 422 naming the missing capability instead of returning prose that breaks the JSON contract. - **#2** - The pinned-model capability check now honors `cfg.routing.require_vision` / `require_json_mode`, so a pin and routed traffic face the same gate. - **#3** - Drop 'image' modality from `deepseek-v4-flash` in opencode.json (`supports_vision = 0`), verified against the live catalog. - **#4** - README hard-filter count Four -> Six. - **#5** - Extract `routing.capability_gate_reason()`, shared by `rejection_reason` and the pinned check, so the flag rule cannot drift between the two. - **#6** - Single `iter_image_url_values()` generator replaces four image-part traversals; `_run_local_vision` computes count/bytes once. ## Tests Adds 9 tests (capability_gate_reason unit coverage, json-mode-gated local fallback, vision-gate-off for pins). Full suite: 416 passing (was 407).
alee added 3 commits 2026-08-23 17:03:56 +00:00
local_vision.enabled now defaults to true (config.yaml and the Pydantic
default both), documented and tested as the shipped behavior rather than an
opt-in.

chat_completions only stripped a client's `provider/model` prefix (opencode
sends `llm-router/...` even for a pin, not just `auto`) when the bare name
was the routing sentinel. A pinned model kept its prefix past that point, so
_check_pinned_capabilities looked up a model_id the catalog never heard of
and fail-closed a genuinely capable pin into a false 422 -- and had it
passed, the same unstripped id would have gone upstream and drawn a 400 from
NeuralWatt, which only knows the bare form. Now the bare suffix is trusted
whenever it resolves to a real catalog row, for a pin as much as for auto.
6 open items from the /code-review medium run: the local-vision fallback
ignoring require_json_mode, the pinned-model gate ignoring
require_vision/require_json_mode config, opencode.json advertising vision
deepseek-v4-flash doesn't have, a README filter-count typo, and two
duplication cleanups (the pinned check re-implementing rejection_reason's
capability rule, and image-part traversal repeated four times). Each entry
has the concrete fix and a test to add; no code changes in this commit.
Implements the bugs_to_fix/capability-gate-followups.md plan (#1, #2, #3,
#4, #5, #6).

- #1: local vision fallback honors require_json_mode - an image + json_object
  request now falls through to the 422 naming the missing capability instead
  of returning prose that breaks the json contract
- #2: the pinned-model capability check now honors cfg.routing.require_vision /
  require_json_mode, so a pin and routed traffic face the same gate
- #3: drop 'image' modality from deepseek-v4-flash (supports_vision = 0)
- #4: README hard-filter count Four -> Six
- #5: extract routing.capability_gate_reason() shared by rejection_reason and
  the pinned check, so the flag rule cannot drift between the two
- #6: single iter_image_url_values() generator replaces four image-part
  traversals; _run_local_vision computes count/bytes once

Adds 9 tests (capability_gate_reason unit coverage, json-mode-gated local
fallback, vision gate off for pins). Full suite: 416 passing.
alee added 1 commit 2026-08-23 17:49:28 +00:00
Reviewing PR #3's iter_image_url_values (which correctly collapsed four
traversals into one, per the capability-gate-followups plan): a part typed
image_url but carrying neither a nested image_url.url nor a bare url key
now yields nothing at all instead of an empty string, so it vanished from
every derived check instead of failing them. Confirmed live before this
fix: has_images read False (dodging the vision gate) and
_local_vision_data_uris_ok read True (an SSRF guard passing vacuously on
zero yielded items) for exactly this shape -- both regressions from what
the four original hand-rolled loops did.

Now every image_url-typed part yields exactly one string, "" when nothing
resolves, which fails startswith("data:") and still counts toward
any()/sum() -- matching pre-PR behavior. Two regression tests added.

Also renames bugs_to_fix/ to code_reviews/, since this is the second report
that's gone in there and "bugs to fix" undersold what the first one turned
into -- and adds a review report for PR #3 alongside the plan it was
implementing.
alee added 1 commit 2026-08-23 18:19:52 +00:00
Generalizes the lesson from the iter_image_url_values fail-open regression
(code_reviews/pr-3-capability-gate-followups-review.md): a plan that
proposes collapsing N implementations into one shared helper has to specify
what it yields on every input shape, including the "well-typed but
unparseable" case that the N originals never had to agree on explicitly.
Gives a contract-table template for future plans to use, and a watch list
of dispatcher.py traversals that aren't currently merged/broken but would
need the same treatment if they ever are.

Starts code_plans/ as where forward-looking spec/plan reports go, alongside
code_reviews/ for after-the-fact review reports. No code changes.
alee added 1 commit 2026-08-23 20:02:27 +00:00
The shared helper's fail-closed behavior (yielding the "" sentinel rather
than skipping a well-typed-but-unparseable image_url part) was only tested
indirectly through detect_capabilities. Add direct unit tests covering the
four input shapes of the shared-helper-unparseable-input-contract plan:
common/valid, alternate valid, well-typed-but-unparseable (-> sentinel), and
wrong-typed/absent (-> skip). Presence of the image_url key is the type tag:
a present-but-malformed payload still yields the sentinel.
alee merged commit b4dcbb86ba into main 2026-08-23 20:02:41 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: alee/6krrt#3