Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 409
fix: stop treating a model list as an allowlist#3330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
21 changes: 21 additions & 0 deletions
21 apps/desktop/src/renderer/locales/settings-provider-copy.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
152 changes: 152 additions & 0 deletions
152 apps/desktop/src/renderer/settings/provider-add-model-dialog.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| import { useState, type FormEvent } from 'react'; | ||
| import { Dialog, DialogHeader } from '@astryxdesign/core/Dialog'; | ||
| import { FormLayout } from '@astryxdesign/core/FormLayout'; | ||
| import { Layout, LayoutContent, LayoutFooter } from '@astryxdesign/core/Layout'; | ||
| import { Button, HStack, NumberInput, TextInput, useUiLocale } from '@maka/ui'; | ||
| import { getProviderSettingsCopy } from '../locales/settings-provider-copy'; | ||
| /** | ||
| * Introduce a model by exact id, for a provider whose catalog cannot grow on | ||
| * its own: without a model-list endpoint, refresh replays the array this build | ||
| * shipped, so a model the user's plan already serves has no other way in | ||
| * (#1584). | ||
| * | ||
| * Two fields. The id enters `enabledModelIds`, which is the authorization. The | ||
| * context window is the one fact nothing else can supply: an id Maka has never | ||
| * seen resolves no window, and the history budget falls back to a flat 32k | ||
| * (context-budget-policy.ts) — three percent of a 1M-token window. | ||
| * | ||
| * Everything else a user can declare is edited in the capability section below | ||
| * the model list, which shows a row for exactly the models Maka cannot | ||
| * describe — every model added here, the moment it is added. | ||
| */ | ||
| export function AddModelDialog(props: { | ||
| isOpen: boolean; | ||
| existingModelIds: readonly string[]; | ||
| /** Another write is in flight; the store would drop this one on the floor. */ | ||
| isSubmitDisabled?: boolean; | ||
| onOpenChange(open: boolean): void; | ||
| /** Resolves to whether the write landed; the draft is held until it did. */ | ||
| onSubmit(id: string, contextWindow: number): Promise<boolean>; | ||
| }) { | ||
| const copy = getProviderSettingsCopy(useUiLocale()).detail; | ||
| const [id, setId] = useState(''); | ||
| const [contextWindow, setContextWindow] = useState<number | null>(null); | ||
| const [submitAttempted, setSubmitAttempted] = useState(false); | ||
| const [isSaving, setSaving] = useState(false); | ||
| const trimmedId = id.trim(); | ||
| const idError = !trimmedId | ||
| ? copy.addModelIdRequired | ||
| : props.existingModelIds.includes(trimmedId) | ||
| ? copy.addModelIdDuplicate | ||
| : null; | ||
| // Required, not defaulted: an unknown window falls back to a flat 32k history | ||
| // budget, and guessing higher on the user's behalf would trade a wasted | ||
| // window for requests the provider rejects outright. Whoever types an exact | ||
| // model id is reading the provider's own model page, where this is stated. | ||
| const contextWindowError = contextWindow ? null : copy.addModelContextWindowRequired; | ||
| function close() { | ||
| setId(''); | ||
| setContextWindow(null); | ||
| setSubmitAttempted(false); | ||
| props.onOpenChange(false); | ||
| } | ||
| // Closing on submit would clear the draft before the write settles, and an | ||
| // exact model id is not something a user can reproduce from memory. The | ||
| // failure is reported by the caller's toast; what this owes them is the | ||
| // typed text, still there to retry from. | ||
| async function submit(event: FormEvent) { | ||
| event.preventDefault(); | ||
| setSubmitAttempted(true); | ||
| if (idError || !contextWindow || isSaving) return; | ||
| setSaving(true); | ||
| try { | ||
| if (await props.onSubmit(trimmedId, contextWindow)) close(); | ||
| } finally { | ||
| setSaving(false); | ||
| } | ||
| } | ||
| return ( | ||
| <Dialog | ||
| isOpen={props.isOpen} | ||
| onOpenChange={(open) => { | ||
| // A write in flight owns the draft until it settles: dismissing here | ||
| // would discard the very text the retry needs. | ||
| if (!open && !isSaving) close(); | ||
| }} | ||
| purpose="form" | ||
| width={480} | ||
| > | ||
| <Layout | ||
| header={ | ||
| <DialogHeader | ||
| title={copy.addModel} | ||
| onOpenChange={(open) => { | ||
| if (!open && !isSaving) close(); | ||
| }} | ||
| /> | ||
| } | ||
| content={ | ||
| <LayoutContent> | ||
| <form id="maka-add-model-form" onSubmit={(event) => void submit(event)}> | ||
| <FormLayout> | ||
| {/* The exact id, kept verbatim through selection and inference | ||
| — `deepseek-v4-pro-beta` is a different model from | ||
| `deepseek-v4-pro`, and only the user knows which one their | ||
| plan actually serves. */} | ||
| <TextInput | ||
| label={copy.addModelIdField} | ||
| description={copy.addModelIdFieldHelp} | ||
| isRequired | ||
| hasAutoFocus | ||
| value={id} | ||
| placeholder={copy.addModelIdPlaceholder} | ||
| onChange={setId} | ||
| status={ | ||
| submitAttempted && idError ? { type: 'error', message: idError } : undefined | ||
| } | ||
| /> | ||
| <NumberInput | ||
| label={copy.addModelContextWindow} | ||
| description={copy.addModelContextWindowHelp} | ||
| isRequired | ||
| value={contextWindow} | ||
| hasClear | ||
| isIntegerOnly | ||
| min={1} | ||
| onChange={setContextWindow} | ||
| status={ | ||
| submitAttempted && contextWindowError | ||
| ? { type: 'error', message: contextWindowError } | ||
| : undefined | ||
| } | ||
| /> | ||
| </FormLayout> | ||
| </form> | ||
| </LayoutContent> | ||
| } | ||
| footer={ | ||
| <LayoutFooter> | ||
| {/* One button, as in scheduled-task-form-dialog: the header's close | ||
| control and Escape are already two ways out, so a footer cancel | ||
| would be a third route to the same place. */} | ||
| <HStack gap={2} hAlign="end"> | ||
| <Button | ||
| variant="primary" | ||
| type="submit" | ||
| form="maka-add-model-form" | ||
| isDisabled={props.isSubmitDisabled || isSaving} | ||
| isLoading={isSaving} | ||
| label={copy.addModelConfirm} | ||
| /> | ||
| </HStack> | ||
| </LayoutFooter> | ||
| } | ||
| /> | ||
| </Dialog> | ||
| ); | ||
| } | ||
57 changes: 50 additions & 7 deletions
57 apps/desktop/src/renderer/settings/provider-connection-detail.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66 apps/desktop/src/renderer/settings/use-connection-detail.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -244,6 +244,7 @@ export function useConnectionDetail(props: ConnectionDetailProps) { | ||
| slug: connection.slug, | ||
| providerType: connection.providerType, | ||
| defaultModel: connection.defaultModel, | ||
| enabledModelIds, | ||
| models: modelSource === 'fetched' || models.length > 0 ? models : undefined, | ||
| modelSource, | ||
| modelsFetchedAt: connection.modelsFetchedAt, | ||
| @@ -484,6 +485,70 @@ export function useConnectionDetail(props: ConnectionDetailProps) { | ||
| } | ||
| } | ||
| /** | ||
| * Introduce a model the provider's catalog does not list. | ||
| * | ||
| * Only offered where refresh cannot help: a provider with no model-list | ||
| * endpoint replays the array this build shipped, so a model the user's plan | ||
| * serves but Maka has never heard of has no other way in (#1584). | ||
| * | ||
| * The id enters `enabledModelIds` — the same user-selection authority a | ||
| * catalogued model uses, so nothing here pretends the provider advertised | ||
| * it — and the context window enters `relayModelProfiles`, which is where a | ||
| * user states a fact no other source knows. Both go in ONE write: the store | ||
| * requires every declaration to key an enabled model, so a table written | ||
| * ahead of its id would be rejected. | ||
| * | ||
| * The saved table is the base, not the unsaved draft: adding a model must | ||
| * not silently commit edits the user has open in the capability section. | ||
| * The draft is then caught up by hand, because a dirty draft deliberately | ||
| * does not reseed from props — see `relayProfileDraftReseedPlan`. | ||
| */ | ||
| async function addDeclaredModel(id: string, contextWindow: number): Promise<boolean> { | ||
| const modelId = id.trim(); | ||
| if (!modelId || enabledModelIds.includes(modelId)) return false; | ||
| if (connectionDetailActionGuard.has('save-enabled-models') || detailActionBusy) return false; | ||
| const next = [...enabledModelIds, modelId]; | ||
| const previous = enabledModelIds; | ||
| const lifecycle = connectionDetailLifecycleRef.current; | ||
| const releaseSaveModels = connectionDetailActionGuard.begin('save-enabled-models'); | ||
| if (!releaseSaveModels) return false; | ||
| setSavingEnabledModels(true); | ||
| setEnabledModelIds(next); | ||
| let saved = false; | ||
| try { | ||
| await props.bridge.update(connection.slug, { | ||
| enabledModelIds: next, | ||
| relayModelProfiles: { ...(savedRelayProfiles ?? {}), [modelId]: { contextWindow } }, | ||
Astro-Han marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| }); | ||
| saved = true; | ||
| if (!isConnectionDetailCurrent(lifecycle)) return saved; | ||
| // The editor's draft is a second copy of this table, and while it is | ||
| // dirty it does not reseed from props — that is what keeps an unrelated | ||
| // reload from discarding typed work. So the declaration just written has | ||
| // to be merged in here. Without it the draft is a table that no longer | ||
| // contains this model, the capability-save button lights up on that | ||
| // difference, and its whole-table replace drops the context window the | ||
| // user just declared — silently, back to the unknown-model default. | ||
| setRelayProfileDrafts((current) => ({ ...current, [modelId]: { contextWindow } })); | ||
| await props.onChanged(); | ||
| } catch (error) { | ||
| if (!isConnectionDetailCurrent(lifecycle)) return saved; | ||
| if (!saved) setEnabledModelIds(previous); | ||
| toast.error( | ||
| saved ? copy.refreshFailed : copy.saveModelsFailed, | ||
| providerPanelActionErrorMessage(error, locale), | ||
| ); | ||
| } finally { | ||
| releaseSaveModels(); | ||
| if (isConnectionDetailCurrent(lifecycle)) setSavingEnabledModels(false); | ||
| } | ||
| // Whether the write landed. The dialog holds the typed id and context | ||
| // window until it did: a rejected write leaves nothing to retype from, and | ||
| // an exact model id is not something a user can reproduce from memory. | ||
| return saved; | ||
| } | ||
| async function runTest() { | ||
| const releaseTest = connectionDetailActionGuard.beginExclusive('test'); | ||
| if (!releaseTest) return; | ||
| @@ -653,6 +718,7 @@ export function useConnectionDetail(props: ConnectionDetailProps) { | ||
| lastTestAtMs, | ||
| save, | ||
| updateEnabledModels, | ||
| addDeclaredModel, | ||
| relayProfileDraft: relayProfileDrafts, | ||
| relayProfilesDirty, | ||
| hasRelayProfileChanges, | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.