Skip to content

Commit 817b92f

Browse files
only21milmaria-rcksjuliusmarminge
authored
fix(pi): stop offering extension-only models for text generation (#24110)
Co-authored-by: maria-rcks <[email protected]> Co-authored-by: Julius Marminge <[email protected]>
1 parent beaa647 commit 817b92f

12 files changed

Lines changed: 371 additions & 64 deletions

File tree

‎apps/server/src/textGeneration/CodexTextGeneration.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,8 @@ const PREFLIGHT_PRIMARY_STATES = [
546546
"not installed, fallback disabled for project",
547547
"signed out",
548548
"signed out, fallback failed",
549+
"cannot generate text",
550+
"model cannot generate text",
549551
] as const;
550552

551553
it.effect.each(PREFLIGHT_PRIMARY_STATES)("text generation preflight: %s", (primaryState) =>
@@ -590,6 +592,22 @@ it.effect.each(PREFLIGHT_PRIMARY_STATES)("text generation preflight: %s", (prima
590592
...(primaryState === "signed out, fallback failed" && instanceId === fallback
591593
? { status: "error", auth: { status: "unknown" } }
592594
: {}),
595+
...(primaryState === "cannot generate text" && instanceId === primary
596+
? { supportsTextGeneration: false }
597+
: {}),
598+
...(primaryState === "model cannot generate text" && instanceId === primary
599+
? {
600+
models: [
601+
{
602+
slug: selection.model,
603+
name: selection.model,
604+
isCustom: false,
605+
supportsTextGeneration: false,
606+
capabilities: null,
607+
},
608+
],
609+
}
610+
: {}),
593611
...(primaryState === "exhausted" && instanceId === primary
594612
? {
595613
usageLimits: {

‎apps/server/src/textGeneration/TextGeneration.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,11 @@ export const make = Effect.gen(function* () {
108108
const unusable =
109109
textGeneratorCannotRun ||
110110
(selected?.enabled === true &&
111-
selected.status === "error" &&
112-
(!selected.installed || selected.auth.status === "unauthenticated"));
111+
(!isProviderTextGenerationCapable(selected) ||
112+
selected.models.find((model) => model.slug === selection.model)
113+
?.supportsTextGeneration === false ||
114+
(selected.status === "error" &&
115+
(!selected.installed || selected.auth.status === "unauthenticated"))));
113116
if (unusable || exhaustedUsageWindow(selected, now, selection.model)) {
114117
const settings = yield* serverSettings.getSettings.pipe(
115118
Effect.mapError(

‎apps/web/src/components/settings/SettingsPanels.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ import { SnoozePresetsRow } from "./SnoozePresetsSettings";
6767
import { useDesktopUpdateState } from "../../state/desktopUpdate";
6868
import { managedTelemetryOffMessage, usePrimaryManagedPolicy } from "../../state/managedPolicy";
6969
import {
70-
getCustomModelOptionsByInstance,
70+
getTextGenerationModelOptionsByInstance,
7171
resolveAppModelSelectionState,
7272
selectsPlanAgent,
7373
} from "../../modelSelection";
@@ -1743,7 +1743,7 @@ export function ThreadsSettingsPanel() {
17431743
);
17441744
const textGenProvider: ProviderDriverKind =
17451745
textGenInstanceEntry?.driverKind ?? DEFAULT_DRIVER_KIND;
1746-
const textGenerationModelOptionsByInstance = getCustomModelOptionsByInstance(
1746+
const textGenerationModelOptionsByInstance = getTextGenerationModelOptionsByInstance(
17471747
settings,
17481748
textGenerationProviders,
17491749
textGenInstanceId,
@@ -1756,6 +1756,7 @@ export function ThreadsSettingsPanel() {
17561756
const textGenerationModelDisabledReason = useScopedModelDisabledReason(
17571757
settings,
17581758
textGenerationModelInstanceEntries,
1759+
true,
17591760
);
17601761
const mixedAddProjectBaseDirectory = useScopedSettingsMixed(["addProjectBaseDirectory"]);
17611762
const mixedTextGenerationModel = useScopedSettingsMixed(["textGenerationModelSelection"]);

‎apps/web/src/components/settings/SourceControlWritingSettings.tsx‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { useSettingsScope } from "./SettingsScopeContext";
2323
import { providerInstanceEntriesFor } from "../../providerInstances";
2424
import { useTextGenerationTargets } from "../../textGenerators";
2525
import {
26-
getCustomModelOptionsByInstance,
26+
getTextGenerationModelOptionsByInstance,
2727
resolveAppModelSelectionState,
2828
selectsPlanAgent,
2929
} from "../../modelSelection";
@@ -151,13 +151,13 @@ export function SourceControlWritingSettingsSection() {
151151
(entry) =>
152152
entry.instanceId === defaultModelSelection.instanceId && entry.enabled && entry.isAvailable,
153153
);
154-
const modelOptionsByInstance = getCustomModelOptionsByInstance(
154+
const modelOptionsByInstance = getTextGenerationModelOptionsByInstance(
155155
settings,
156156
projectEffectiveTextGenerationProviders,
157157
activeSelection.instanceId,
158158
activeSelection.model,
159159
);
160-
const writerModelDisabledReason = useScopedModelDisabledReason(settings, writerEntries);
160+
const writerModelDisabledReason = useScopedModelDisabledReason(settings, writerEntries, true);
161161
const writerInstanceEntry = writerEntries.find(
162162
(entry) => entry.instanceId === activeSelection.instanceId,
163163
);
@@ -179,7 +179,7 @@ export function SourceControlWritingSettingsSection() {
179179
},
180180
projectAgentProviders,
181181
);
182-
const reviewGuideModelOptionsByInstance = getCustomModelOptionsByInstance(
182+
const reviewGuideModelOptionsByInstance = getTextGenerationModelOptionsByInstance(
183183
settings,
184184
projectAgentProviders,
185185
reviewGuideSelection.instanceId,
@@ -595,7 +595,21 @@ export function SourceControlWritingSettingsSection() {
595595
<Switch
596596
checked={usesDedicatedModel}
597597
disabled={!usesDedicatedModel && !canEnableDedicatedModel}
598-
onCheckedChange={(checked) =>
598+
onCheckedChange={(checked) => {
599+
if (checked) {
600+
const reason = writerModelDisabledReason(
601+
defaultModelSelection.instanceId,
602+
defaultModelSelection.model,
603+
);
604+
if (reason) {
605+
toastManager.add({
606+
type: "error",
607+
title: "Writer model not saved",
608+
description: reason,
609+
});
610+
return;
611+
}
612+
}
599613
updateSettings({
600614
sourceControlWriterModelSelection: checked
601615
? createModelSelection(
@@ -604,8 +618,8 @@ export function SourceControlWritingSettingsSection() {
604618
defaultModelSelection.options,
605619
)
606620
: null,
607-
})
608-
}
621+
});
622+
}}
609623
aria-label="Use a separate source control writer model"
610624
/>
611625
</div>

‎apps/web/src/components/settings/TextGeneratorsSettings.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import {
2828
textGeneratorBrand,
2929
textGeneratorDisplayName,
3030
} from "../../textGenerators";
31-
import { getCustomModelOptionsByInstance } from "../../modelSelection";
31+
import { getTextGenerationModelOptionsByInstance } from "../../modelSelection";
3232
import { ProviderInstanceIcon } from "../chat/ProviderInstanceIcon";
3333
import { ProviderModelPicker } from "../chat/ProviderModelPicker";
3434
import {
@@ -563,7 +563,7 @@ function TextGenerationTaskPicker({
563563
model={selection.model}
564564
lockedProvider={null}
565565
instanceEntries={targets.entries}
566-
modelOptionsByInstance={getCustomModelOptionsByInstance(
566+
modelOptionsByInstance={getTextGenerationModelOptionsByInstance(
567567
settings,
568568
targets.providers,
569569
selection.instanceId,

‎apps/web/src/components/settings/VoiceSettingsPanel.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ import {
7272
import { updateClientSettings, useClientSettings } from "../../hooks/useSettings";
7373
import { runEnvironmentSpeech } from "../../speech/runEnvironmentSpeech";
7474
import {
75-
getCustomModelOptionsByInstance,
75+
getTextGenerationModelOptionsByInstance,
7676
resolveAppModelSelectionState,
7777
} from "../../modelSelection";
7878
import { useTextGenerationTargets } from "../../textGenerators";
@@ -531,14 +531,14 @@ function CleanupModelRow() {
531531
},
532532
textGenerationProviders,
533533
);
534-
const modelOptionsByInstance = getCustomModelOptionsByInstance(
534+
const modelOptionsByInstance = getTextGenerationModelOptionsByInstance(
535535
settings,
536536
textGenerationProviders,
537537
selection.instanceId,
538538
selection.model,
539539
);
540540
const instanceEntry = instanceEntries.find((entry) => entry.instanceId === selection.instanceId);
541-
const modelDisabledReason = useScopedModelDisabledReason(settings, instanceEntries);
541+
const modelDisabledReason = useScopedModelDisabledReason(settings, instanceEntries, true);
542542
const setModel = (next: ReturnType<typeof createModelSelection>) => {
543543
const reason = modelDisabledReason(next.instanceId, next.model);
544544
if (reason) {

‎apps/web/src/components/settings/useScopedModelAvailability.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import {
55
} from "@t3tools/contracts";
66

77
import { mergeEnvironmentSettings } from "../../hooks/useSettings";
8-
import { getCustomModelOptionsByInstance } from "../../modelSelection";
8+
import {
9+
getCustomModelOptionsByInstance,
10+
getTextGenerationModelOptionsByInstance,
11+
} from "../../modelSelection";
912
import {
1013
applyProviderInstanceSettings,
1114
deriveProviderInstanceEntries,
@@ -17,6 +20,7 @@ import { useSettingsScope } from "./SettingsScopeContext";
1720
export function useScopedModelDisabledReason(
1821
settings: UnifiedSettings,
1922
entries: readonly ProviderInstanceEntry[],
23+
textGeneration = false,
2024
) {
2125
const { targets } = useSettingsScope();
2226
const { environments } = useEnvironments();
@@ -48,7 +52,10 @@ export function useScopedModelDisabledReason(
4852
candidate.settings,
4953
candidate.projectId,
5054
).find((option) => option.instanceId === instanceId);
51-
const options = getCustomModelOptionsByInstance(
55+
const optionsForInstance = textGeneration
56+
? getTextGenerationModelOptionsByInstance
57+
: getCustomModelOptionsByInstance;
58+
const options = optionsForInstance(
5259
mergeEnvironmentSettings(candidate.settings, settings),
5360
providers,
5461
).get(instanceId);

‎apps/web/src/modelSelection.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import {
2525
import {
2626
defaultModelSelectionsAreMixed,
2727
getAppModelOptionsForInstance,
28+
getCustomModelOptionsByInstance,
29+
getTextGenerationModelOptionsByInstance,
2830
resolveAppModelSelectionForInstance,
2931
resolveAppModelSelectionState,
3032
resolveEffectiveDefaultModelSelection,
@@ -917,6 +919,60 @@ describe("instance-scoped model selection", () => {
917919
NO_PROVIDER_MODEL_SELECTION,
918920
);
919921
});
922+
923+
it("excludes customized extension models while preserving unknown custom and thread models", () => {
924+
const pi = provider({
925+
provider: ProviderDriverKind.make("pi"),
926+
instanceId: "pi",
927+
models: ["default", "demo/demo-model"],
928+
});
929+
const providers = [
930+
{
931+
...pi,
932+
models: pi.models.map((model) =>
933+
model.slug === "demo/demo-model" ? { ...model, supportsTextGeneration: false } : model,
934+
),
935+
},
936+
];
937+
const settings: UnifiedSettings = {
938+
...DEFAULT_UNIFIED_SETTINGS,
939+
providerInstances: {
940+
[pi.instanceId]: {
941+
driver: ProviderDriverKind.make("pi"),
942+
config: {
943+
customModels: [
944+
{ slug: "demo/demo-model", name: "My extension model" },
945+
{ slug: "unknown/model", name: "My custom model" },
946+
],
947+
},
948+
},
949+
},
950+
textGenerationModelSelection: createModelSelection(pi.instanceId, "demo/demo-model"),
951+
};
952+
const supported = providers.filter((provider) => provider.supportsTextGeneration !== false);
953+
expect(
954+
getTextGenerationModelOptionsByInstance(settings, supported, pi.instanceId, "demo/demo-model")
955+
.get(pi.instanceId)
956+
?.map((option) => option.slug),
957+
).toEqual(["default", "unknown/model"]);
958+
expect(resolveAppModelSelectionState(settings, supported)).toEqual(
959+
createModelSelection(pi.instanceId, "default"),
960+
);
961+
expect(
962+
resolveAppModelSelectionState(
963+
{
964+
...settings,
965+
textGenerationModelSelection: createModelSelection(pi.instanceId, "unknown/model"),
966+
},
967+
supported,
968+
).model,
969+
).toBe("unknown/model");
970+
expect(
971+
getCustomModelOptionsByInstance(settings, providers)
972+
.get(pi.instanceId)
973+
?.map((option) => option.slug),
974+
).toEqual(["default", "demo/demo-model", "unknown/model"]);
975+
});
920976
});
921977

922978
describe("resolveAppModelSelectionState with the opencode plan agent", () => {

‎apps/web/src/modelSelection.ts‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,42 @@ export function getCustomModelOptionsByInstance(
366366
return out;
367367
}
368368

369+
function getTextGenerationModelOptionsForInstance(
370+
settings: UnifiedSettings,
371+
entry: ProviderInstanceEntry,
372+
selectedModel?: string | null,
373+
): AppModelOption[] {
374+
const unsupported = new Set(
375+
entry.models
376+
.filter((model) => model.supportsTextGeneration === false)
377+
.map((model) => model.slug),
378+
);
379+
// Filter after custom entries are rebuilt so a display-name override cannot restore a model.
380+
return getAppModelOptionsForInstance(settings, entry, selectedModel).filter(
381+
(option) => !unsupported.has(option.slug),
382+
);
383+
}
384+
385+
export function getTextGenerationModelOptionsByInstance(
386+
settings: UnifiedSettings,
387+
providers: ReadonlyArray<ServerProvider>,
388+
selectedInstanceId?: ProviderInstanceId | null,
389+
selectedModel?: string | null,
390+
): ReadonlyMap<ProviderInstanceId, ReadonlyArray<ModelEsque>> {
391+
return new Map(
392+
deriveProviderInstanceEntries(
393+
providers.filter((provider) => provider.supportsTextGeneration !== false),
394+
).map((entry) => [
395+
entry.instanceId,
396+
getTextGenerationModelOptionsForInstance(
397+
settings,
398+
entry,
399+
entry.instanceId === selectedInstanceId ? selectedModel : null,
400+
),
401+
]),
402+
);
403+
}
404+
369405
export function selectsPlanAgent(options: ModelSelection["options"]): boolean {
370406
return options?.some((option) => option.id === "agent" && option.value === "plan") ?? false;
371407
}
@@ -390,21 +426,17 @@ export function resolveAppModelSelectionState(
390426
if (entry) {
391427
const selectedModel = selectedEntry ? selection.model : null;
392428
// Use the cheap generation default; the catalog default is the expensive chat flagship.
393-
const options = getAppModelOptionsForInstance(settings, entry);
429+
const options = getTextGenerationModelOptionsForInstance(settings, entry);
394430
const model =
395431
resolveSelectableModel(entry.driverKind, selectedModel, options) ??
396432
resolveSelectableModel(
397433
entry.driverKind,
398434
DEFAULT_TEXT_GENERATION_MODEL_BY_PROVIDER[entry.driverKind],
399435
options,
400436
) ??
401-
resolveAppModelSelectionForInstance(
402-
entry.instanceId,
403-
settings,
404-
supportedProviders,
405-
selectedModel,
406-
) ??
407-
entry.models[0]?.slug ??
437+
options.find((option) => option.isDefault)?.slug ??
438+
options[0]?.slug ??
439+
entry.models.find((model) => model.supportsTextGeneration !== false)?.slug ??
408440
DEFAULT_TEXT_GENERATION_MODEL_BY_PROVIDER[entry.driverKind];
409441
if (!model) {
410442
return createModelSelection(entry.instanceId, "", []);

‎packages/contracts/src/server.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ export const ServerProviderModel = Schema.Struct({
8383
isCustom: Schema.Boolean,
8484
isDefault: Schema.optional(Schema.Boolean),
8585
isLegacy: Schema.optional(Schema.Boolean),
86+
// False when the provider can run this model in sessions but not for
87+
// application text generation. Absent means supported.
88+
supportsTextGeneration: Schema.optional(Schema.Boolean),
8689
capabilities: Schema.NullOr(ModelCapabilities),
8790
});
8891
export type ServerProviderModel = typeof ServerProviderModel.Type;

0 commit comments

Comments
 (0)