Skip to content

fix(core): refuse models whose declared output has no text - #4243

Merged
Astro-Han merged 1 commit into
apache:mainfrom
Joob1n:fix/image-only-models-unsupported-for-chat
Aug 30, 2026
Merged

Astro-Han merged 1 commit into
apache:mainfrom
Joob1n:fix/image-only-models-unsupported-for-chat

Conversation

@Joob1n

@Joob1n Joob1n commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

isModelExplicitlyUnsupportedForChat guards four call sites — the model catalog, connection readiness, the execution model authority, and session catalog selection — and could not fire on real data.

Its image branch keys on capabilities.imageGeneration. Nothing in production has ever set that flag; the generated metadata carries the fact as modalities.output instead:

"imageGeneration": true in model-metadata.generated.ts    →   0 entries
declared output that is not text                          → 128 entries
production assignments of imageGeneration                 →   0
assignments in tests                                      →   8

So an image or speech model was selectable as a chat model, and the failure surfaced as a provider error on the first request instead of a refusal at selection. Found while diagnosing a relay connection whose only enabled model was gpt-image-2: the connection test posted a text probe to /v1/responses and the toast could only suggest checking the key, the URL, and the proxy — all three of which were fine.

Three changes:

  • The guard also refuses a model whose declared output names modalities but not text. That is the form the fact actually arrives in. Only an explicit chat: true outranks it; reasoning and functionCalling do not, because a TTS model carrying reasoning: true is describing how it composes speech and still cannot answer in text.
  • makeEntry passes the merged modalities into the availability derivation, beside the merged capabilities. It passed only normalizedModel.modalities, so a bundled image-only model reached the guard with no output declaration at all.
  • toModelInfo stops dropping output_modalities. It was validated by assertOptionalArray and then discarded, so a relay that advertised an image-only model handed back a row indistinguishable from a chat model's.

An empty output list stays allowed. modalities.output is typed to text, image, and audio, so a video model's real output has no representation and serializes as [] — the same shape a generator bug would produce. Blocking on it would be guessing, so the rule reads only non-empty lists. gemini-omni-flash-preview and minimax/minimax-h3 are the concrete cases: both are video models whose output serializes empty, and both stay selectable. Expressing video in the modality union is a separate data question and is not attempted here.

The fetched fact is recorded as a capability rather than as modalities on purpose: decodeConnectionModel is an exactRecord over id, displayName, apiProtocol, contextWindow, maxOutputTokens, capabilities and does not persist modalities, so emitting it would throw unknown field on the next catalog read. chat and imageGeneration are both already in that allowlist.

Behavior change

Models that declare a non-text output are now refused at selection rather than at the first request. Concretely, on bundled metadata that is every image model (gpt-image-*, bytedance/seedream-*, bfl/flux-*, recraft/*, …) and every TTS model (openai/tts-*, gemini-*-tts, fish-audio/*, mimo-*-tts, …). I checked all 128 entries whose declared output is not text: every one is an image, speech, or video generation model, and none is a chat model.

A user who had already selected such a model as their default sees model_not_chat_capable instead of a provider error whose text depends on the relay.

Verification

packages/core         712 tests, 712 pass, 0 fail
packages/runtime-host 1406 tests, 1397 pass, 0 fail
packages/desktop      1686 tests, 1686 pass, 0 fail
packages/runtime      3081 tests, 3063 pass, 5 fail  (see below)
typecheck (core, runtime)   clean
biome check                 clean
npm run check:model-metadata  clean — no regeneration, the generated file is untouched

The 5 packages/runtime failures are spawn rg ENOENT — ripgrep is not installed on this machine. I confirmed they fail identically on this branch's merge-base with main and with these changes stashed, and all five are Glob/Grep tests untouched by this PR.

Each change fails a test when reverted individually:

reverted result
the modality rule in the guard model-catalog 16 pass / 1 fail
merged modalities into deriveModelUnavailableReason model-catalog 16 pass / 1 fail
output_modalities capture in toModelInfo model-fetcher 12 pass / 1 fail
the knownOutputModalities filter model-fetcher 12 pass / 1 fail

Behavior against the real bundled catalog, before and after:

                                out             before   after
openai/gpt-image-2              ["image"]       false    true
openai/gpt-image-1              ["image"]       false    true
gemini-2.5-flash-preview-tts    ["audio"]       false    true
gemini-3.1-flash-tts-preview    ["audio"]       false    true
gemini-omni-flash-preview       []              false    false
openai/gpt-5.2                  ["text"]        false    false
anthropic/claude-opus-4-6       ["text"]        false    false

Self-review

Reviewing the diff before opening this turned up one defect, which is fixed here.

assertOptionalArray validates the container and never its items, so output_modalities is typed string[] but holds whatever the relay returned. My first version read the raw array, which meant ['Text'], [null], or [42] would all fail includes('text') and set chat: falsesilently disabling a working chat model. The neighbouring modality reads in that function do not have this problem because they only ever add a capability, where an unrecognized value merely costs a fact; this one removes chat, so the same miss is destructive. The read now goes through knownOutputModalities, which keeps only values this build understands, so an unrecognized list reads as "said nothing" rather than "said not text". A test covers each of those four inputs plus the mixed ['image', 'hologram'] case.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 via Claude Code — investigation, the three code changes, the tests, and this description. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

`isModelExplicitlyUnsupportedForChat` guards four call sites — the model
catalog, connection readiness, the execution model authority, and session
catalog selection — and could not fire on real data. Its image branch keys
on `capabilities.imageGeneration`, and nothing in production has ever set
that flag: the generated metadata carries the fact as
`modalities.output` instead, where 128 entries declare an output that is
not text and 0 declare `imageGeneration`. So `gpt-image-2` and every other
image or speech model was selectable as a chat model, and the failure
surfaced as a provider error on the first request rather than a refusal at
selection.

The guard now also refuses a model whose declared output names modalities
but not text, which is the form the fact actually arrives in. Only an
explicit `chat: true` outranks it; `reasoning` and `functionCalling` do
not, because a TTS model carrying `reasoning: true` is describing how it
composes speech and still cannot answer in text.

An empty output list stays allowed. `modalities.output` is typed to text,
image, and audio, so a video model's real output has no representation and
serializes as `[]` — the same shape a generator bug would produce. Blocking
on it would be guessing, so the rule reads only non-empty lists.

`makeEntry` now passes the merged `modalities` into the availability
derivation beside the merged `capabilities`. It passed only
`normalizedModel.modalities`, so a bundled image-only model reached the
guard with no output declaration at all.

`toModelInfo` stopped dropping `output_modalities`. It was validated and
discarded, so a relay that advertised an image-only model handed back a row
indistinguishable from a chat model's. The fact is recorded as a
capability rather than as modalities because `decodeConnectionModel` is an
exact record that does not persist `modalities` — emitting it would throw
`unknown field` on the next catalog read.

Generated-by: Claude Opus 5 via Claude Code
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 30, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks—this fixes the problem at the right shared authority rather than adding a picker-only workaround.

The issue is real: production metadata expresses image/audio-only output through modalities.output, while the existing guard was looking primarily at capabilities.imageGeneration, which production data did not populate.

This change extends the existing isModelExplicitlyUnsupportedForChat() predicate used by connection readiness, model catalogs, the Runtime Host execution-model authority, and Session catalog selection. It also stops discovery from dropping the provider’s output-modality fact. The guard remains conservative: only a known, non-empty output declaration without text is rejected, while an explicit chat: true still wins.

That keeps one decision boundary, covers both selection and execution, and does not introduce a parallel model-capability authority. I found no P0–P2 issues on exact head 54b9aa4dc9f05f0e41d695f8a411a80b74391a36; the hosted checks are green.

中文对照

这个修复落在正确的共享权威上,而不是只在某个选择器里打补丁。它让连接检查、模型目录、Runtime Host 执行入口和 Session 目录共用同一个判断,同时保守处理未知或空的输出类型。当前没有发现 P0–P2,可以批准。

AI-assisted review: Codex traced the shared production authority and drafted this review; the maintainer verified the final conclusion.

@Astro-Han
Astro-Han merged commit 1c52241 into apache:main Aug 30, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants