Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions packages/core/src/models/modelConfigResolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -854,4 +854,112 @@ describe('modelConfigResolver', () => {
}
});
});

Comment thread
LaZzyMan marked this conversation as resolved.
describe('[Regression] issue-4219 — env-var-only path must call defaultModalities()', () => {
it('[Regression] env-var-only path: modalities auto-detected for qwen3.6-35b-a3b', () => {
Comment thread
LaZzyMan marked this conversation as resolved.
// REPRODUCES issue-4219:
// When the model is supplied only via OPENAI_MODEL (no modelProviders entry),
// resolveGenerationConfig() iterates MODEL_GENERATION_CONFIG_FIELDS but never
// calls defaultModalities(). This leaves config.modalities undefined, causing
// image attachments to be silently dropped with an "Unsupported <modality>"
// message even though the model supports images.
//
// The modelRegistry path (resolveModelConfig -> resolveModelConfig in modelRegistry.ts)
// and the modelsConfig path (applyResolvedModelDefaults) both call defaultModalities()
// when generationConfig.modalities is undefined. The env-var-only path does not.
const result = resolveModelConfig({
authType: AuthType.USE_OPENAI,
cli: {},
settings: {},
env: {
OPENAI_API_KEY: 'test-key',
OPENAI_BASE_URL: 'http://localhost:8000/v1',
OPENAI_MODEL: 'qwen3.6-35b-a3b',
},
// No modelProvider — this is the env-var-only path
});

expect(result.config.model).toBe('qwen3.6-35b-a3b');

// The qwen3.6-35b pattern in modalityDefaults.ts maps to { image: true, video: true }.
// The env-var-only path must auto-detect this — just as the modelProviders path does
// in modelsConfig.ts applyResolvedModelDefaults() lines 791-797.
Comment thread
LaZzyMan marked this conversation as resolved.
expect(result.config.modalities).toBeDefined();
expect(result.config.modalities?.image).toBe(true);
expect(result.config.modalities?.video).toBe(true);
expect(result.sources['modalities'].kind).toBe('computed');
});

it('env-var-only path: modalities defaults to {} for unknown model (text-only)', () => {
// Locks the invariant: defaultModalities() returns {} (text-only) for
// unknown model patterns, never undefined. After this fix, env-var-only
// setups never re-expose `modalities === undefined` for downstream
// consumers to misinterpret as "unresolved" (issue #4219).
const result = resolveModelConfig({
authType: AuthType.USE_OPENAI,
cli: {},
settings: {},
env: {
OPENAI_API_KEY: 'test-key',
OPENAI_BASE_URL: 'http://localhost:8000/v1',
OPENAI_MODEL: 'some-unknown-model-xyz',
},
});

expect(result.config.modalities).toEqual({});
expect(result.sources['modalities'].kind).toBe('computed');
});

it('Qwen OAuth path: modalities auto-detected for default coder-model', () => {
// resolveGenerationConfig is shared by both the OpenAI and Qwen OAuth
// paths; the latter (resolveQwenOAuthConfig) passes the resolved Qwen
// OAuth model name (defaults to DEFAULT_QWEN_MODEL = 'coder-model') as
// modelId, so the new modalities fallback also fires here.
//
// modalityDefaults.ts maps /^coder-model$/ to { image: true, video: true }
// because the Qwen OAuth coder-model now supports vision (see warning
// text at modelConfigResolver.ts ~L330). This test pins that down so a
// future edit to MODALITY_PATTERNS doesn't silently regress OAuth.
const result = resolveModelConfig({
authType: AuthType.QWEN_OAUTH,
cli: {},
settings: {},
env: {},
});

expect(result.config.model).toBe(DEFAULT_QWEN_MODEL);
expect(result.config.modalities).toEqual({ image: true, video: true });
expect(result.sources['modalities'].kind).toBe('computed');
});

it('env-var-only path: explicit settings.generationConfig.modalities is not overridden by fallback', () => {
// Locks the `=== undefined` guard: when user explicitly configures
// modalities in settings, the fallback must not clobber them with
// defaultModalities() — even for a model whose name would otherwise
// auto-resolve to multimodal.
const result = resolveModelConfig({
authType: AuthType.USE_OPENAI,
cli: {},
settings: {
generationConfig: {
modalities: {
image: false,
pdf: false,
video: false,
audio: false,
},
},
},
env: {
OPENAI_API_KEY: 'test-key',
OPENAI_BASE_URL: 'http://localhost:8000/v1',
OPENAI_MODEL: 'qwen3.6-35b-a3b', // defaultModalities → { image: true, video: true }
},
});

expect(result.config.modalities?.image).toBe(false);
expect(result.config.modalities?.video).toBe(false);
expect(result.sources['modalities'].kind).toBe('settings');
});
});
});
20 changes: 19 additions & 1 deletion packages/core/src/models/modelConfigResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import { AuthType } from '../core/contentGenerator.js';
import type { ContentGeneratorConfig } from '../core/contentGenerator.js';
import { DEFAULT_QWEN_MODEL } from '../config/models.js';
import { defaultModalities } from '../core/modalityDefaults.js';
import {
resolveField,
resolveOptionalField,
Expand Down Expand Up @@ -268,7 +269,7 @@ export function resolveModelConfig(
settings?.generationConfig,
modelProvider?.generationConfig,
authType,
modelProvider?.id,
modelProvider?.id ?? modelResult.value,
Comment thread
LaZzyMan marked this conversation as resolved.
sources,
);

Expand Down Expand Up @@ -396,5 +397,22 @@ function resolveGenerationConfig(
}
}

// modalities fallback: auto-detect from model when neither modelProvider nor
// settings supplied it. Mirrors modelRegistry.resolveModelConfig and
// modelsConfig.applyResolvedModelDefaults so all paths agree on which models
// are multimodal — without this, env-var-only setups silently drop @image
// attachments for image-capable models (issue #4219).
//
// Invariant: defaultModalities() returns `{}` (text-only) for unknown
// models, never `undefined`. After this fallback runs with a known modelId,
// `result.modalities` is always defined. Downstream code must NOT branch
// on `modalities === undefined` to mean "unresolved" — use the sources map
// (kind === 'computed' vs 'modelProviders'/'settings') if that distinction
// matters.
if (result.modalities === undefined && modelId) {
Comment thread
LaZzyMan marked this conversation as resolved.
Comment thread
LaZzyMan marked this conversation as resolved.
result.modalities = defaultModalities(modelId);
sources['modalities'] = computedSource('auto-detected from model');
}

return result;
}
Loading