feat(#3996): resolve image output capability from models.dev - #4019
feat(#3996): resolve image output capability from models.dev#4019aheritier wants to merge 2 commits into
Conversation
ebd9306 to
33c6981
Compare
33c6981 to
5bb315a
Compare
35efc65 to
5bb315a
Compare
Add ModelConfig.OutputCapabilities (output_capabilities.image in YAML), a latest-only tri-state override for a model's generative output capability. An explicit true or false is authoritative; an omitted flag is left for the models.dev resolver introduced by the following commit. No capability is ever inferred from the model name. Reject output_capabilities on first_available selectors because one block cannot unambiguously describe every candidate. Update the schema, model docs, and example, and cover YAML round trips, shorthand behavior, cloning, schema strictness, and selector validation.
Resolve an omitted output_capabilities.image flag from the model's models.dev output modalities, matching modality names case-insensitively. Explicit true or false overrides remain authoritative, while a missing store or catalogue record conservatively disables image output. Wire the resolver through provider base configuration, update schema and model documentation, and cover catalogue hits, misses, nil stores, and override precedence.
5bb315a to
d1bcf0a
Compare
aheritier
left a comment
There was a problem hiding this comment.
Reviewed at head d1bcf0a. CI is green for this SHA (16 successful check-runs, 3 skipped publish jobs, nothing pending; no legacy commit statuses). Locally on the PR head: go build ./... OK, go test -count=1 -race ./pkg/modelinfo/... ./pkg/model/provider/base/... ./pkg/config/... green, golangci-lint (v2.13.2) 0 issues. The new tests are load-bearing — three targeted mutants (dropping the override short-circuit, EqualFold→==, flipping the nil-store default) all fail the suite. Still a draft, so this is a comment rather than an approval.
The mechanics are good: conservative-by-default resolution with a 10s bound, *bool tri-state so an explicit false is authoritative and distinguishable from omitted, deep-copy/round-trip/shorthand behaviour pinned, first_available rejected with an actionable message, frozen pkg/config/v* untouched, strict schema, and examples/gemini_image_output.yaml exercised by TestParseExamples. Note Config.ImageOutputEnabled has no production caller at this head (it's consumed in #4020) — worth a line in the PR body so reviewers don't read it as dead code.
Blocking — the docs in this PR say the opposite of what it does. docs/configuration/models/index.md:154-161 states "There is no automatic detection for output capabilities… always unknown/off unless the owner declares them" (plus :79 "Never inferred" and :176 "Omitting output_capabilities… always preserves existing behavior"), and examples/gemini_image_output.yaml:6-8 says "never inferred from the model name or any catalogue". But pkg/modelinfo/modelinfo.go:645-666 resolves an omitted flag from the models.dev output modalities. d1bcf0a updated the schema descriptions (agent-schema.json:1746, :1780, :1786) but not the prose or the example, though its commit message claims a documentation update.
Blocking — this diverges from #3996's stated contract, and #4020 makes it user-visible. The epic says image generation is "opt-in and owner-declared", that "the configuration remains the authority for image-output eligibility", and that models without the declaration "remain text-only". With catalogue inference, any model listed with an image output modality is opted in with no declaration — and since #4020 gates on the resolved value (config.ResponseModalities = [TEXT, IMAGE], plus the hard rejection "…does not support tools in the same request"), an existing google/gemini-2.5-flash-image + tools setup would go from working to a request-time error. #4020 also reports OutputCapabilityKnown from the declaration while gating on the inferred value, so diagnostics would say "unknown" for requests the guard rejects. Please decide explicitly: keep inference and retract/adjust the epic's owner-declared-only limit (documenting the behaviour change), or keep declared-only and confine the catalogue lookup to diagnostics. Then make code, schema, docs, example and diagnostics agree.
Should-fix:
a205303doesn't compile on its own: its tests referenceResolveOutputImage(pkg/modelinfo/output_capabilities_test.go:40) andConfig.ImageOutputEnabled(pkg/model/provider/base/base_test.go:93), both added only ind1bcf0a—go vet ./pkg/modelinfo/... ./pkg/model/provider/base/...fails at that commit. Moving those test files intod1bcf0arestores atomic/bisectable commits.pkg/config/latest/types.go:131-139addsfunc Bool(value bool) *bool, born deprecated with a//nolint:modernize, and with zero callers repo-wide (the PR's own literals usenew(true)). Looks like a rebase leftover — please drop it.- The new code was inserted between the two lines of
capsFromModalities' doc comment:pkg/modelinfo/modelinfo.go:687now reads// booleans it grants. Unknown modality names are ignored.with the subject line gone.
Optional: adding OutputCapabilitiesConfig to definitionMap in TestSchemaMatchesGoTypes (pkg/config/schema_test.go:307) would guard nested schema drift for future fields; and note that inference-by-default puts every undeclared model on the catalogue path, where a cold cache with models.dev unreachable can cost up to loadCapsTimeout per resolution because the embedded snapshot is deliberately not memoized (pkg/modelsdev/store.go:176-184).
What
Resolves Gemini image-output capability from models.dev, wires it into model/config validation, and documents/examples the capability.
Why
Allows image-output configuration to be validated from authoritative model metadata instead of hard-coded assumptions.
Validation
task test; config, schema, modelinfo, capability-resolution, and output-capability tests.Test instructions
Run:
task testUse
examples/gemini_image_output.yamlwith a Gemini model that advertises image output, then repeat with a model that does not. Expected: the capable model configuration is accepted and the incapable model configuration is rejected with a clear validation error.