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