diff --git a/packages/client/ui-models/src/client/DeepSeekModelsEditor.tsx b/packages/client/ui-models/src/client/DeepSeekModelsEditor.tsx index d80ab9f741..13aab18dab 100644 --- a/packages/client/ui-models/src/client/DeepSeekModelsEditor.tsx +++ b/packages/client/ui-models/src/client/DeepSeekModelsEditor.tsx @@ -82,10 +82,14 @@ export function validateDeepSeekModels(value: unknown): DeepSeekModelsValidation const models = modelDrafts(value) const seen = new Set() for (const [index, model] of models.entries()) { + // Compared trimmed: surrounding whitespace is a paste artifact the adapter + // would never match, and an untrimmed compare lets `model ` slip past the + // duplicate check against its own twin. const id = model['id'] - if (typeof id !== 'string' || id.length === 0) return { index, key: 'modelIdRequired' } - if (seen.has(id)) return { index, key: 'modelIdDuplicate' } - seen.add(id) + const trimmed = typeof id === 'string' ? id.trim() : undefined + if (trimmed === undefined || trimmed.length === 0) return { index, key: 'modelIdRequired' } + if (seen.has(trimmed)) return { index, key: 'modelIdDuplicate' } + seen.add(trimmed) const name = model['name'] if (name !== undefined && (typeof name !== 'string' || name.length === 0)) { return { index, key: 'modelNameInvalid' } @@ -123,10 +127,18 @@ export interface DeepSeekModelsEditorProps { * @returns the catalog editor. */ export function DeepSeekModelsEditor(props: DeepSeekModelsEditorProps): ReactNode { - // The context-window field is edited as text, so the keystrokes are held - // here while one row has focus: re-deriving the text from the parsed count - // on every change would rewrite `1000` to `1K` mid-word. - const [editing, setEditing] = useState<{ index: number; text: string } | undefined>(undefined) + // Context windows are edited as text, so a row's keystrokes are held here + // rather than re-derived from the parsed count on every change, which would + // rewrite `1000` to `1K` mid-word. Unreadable text is kept past blur so the + // save-time rejection names a row the user can still see — which is why + // this is one entry PER ROW: a single active buffer would be displaced by + // editing any other row, and the abandoned row would fall back to rendering + // its stored NaN as the literal `NaN`. + // + // Entries are keyed by row index, so the two operations that move indexes + // maintain them: `remove` re-keys around the dropped row, and reset clears + // them all because the rows they annotated are gone. + const [editing, setEditing] = useState>(() => new Map()) const update = (index: number, key: 'id' | 'name' | 'contextWindow', value: unknown): void => { const next = props.models.map((model, at) => { @@ -140,24 +152,41 @@ export function DeepSeekModelsEditor(props: DeepSeekModelsEditorProps): ReactNod } const remove = (index: number): void => { - setEditing(undefined) + setEditing((current) => { + const next = new Map() + for (const [at, text] of current) { + if (at === index) continue + next.set(at > index ? at - 1 : at, text) + } + return next + }) props.onChange(props.models.filter((_model, at) => at !== index).map(model => ({ ...model }))) } - /** The row's field text: the live keystrokes, else the stored count spelled short. */ + const reset = (): void => { + setEditing(new Map()) + props.onReset() + } + + /** The row's field text: its live keystrokes, else the stored count spelled short. */ const contextText = (model: DeepSeekModelDraft, index: number): string => { - if (editing?.index === index) return editing.text + const typed = editing.get(index) + if (typed !== undefined) return typed const value = model['contextWindow'] return typeof value === 'number' ? formatContextWindow(value) : '' } const settleContext = (index: number): void => { + const typed = editing.get(index) + if (typed === undefined) return + // Unreadable text stays on screen: the save-time rejection names a row the + // user can still see and correct. + const parsed = parseContextWindow(typed) + if (parsed !== undefined && Number.isNaN(parsed)) return setEditing((current) => { - if (current?.index !== index) return current - // Unreadable text stays on screen: the save-time rejection names a row - // the user can still see and correct. - const parsed = parseContextWindow(current.text) - return parsed !== undefined && Number.isNaN(parsed) ? current : undefined + const next = new Map(current) + next.delete(index) + return next }) } @@ -176,7 +205,7 @@ export function DeepSeekModelsEditor(props: DeepSeekModelsEditorProps): ReactNod type="button" className={styles['linkButton']} disabled={props.disabled} - onClick={props.onReset} + onClick={reset} > {props.t('resetModels')} @@ -203,6 +232,12 @@ export function DeepSeekModelsEditor(props: DeepSeekModelsEditorProps): ReactNod aria-label={`${props.t('modelId')} ${String(index + 1)}`} disabled={props.disabled} onChange={(event) => { update(index, 'id', event.target.value) }} + onBlur={(event) => { + // Settle a pasted id rather than trimming per keystroke, + // which would stop the user typing an interior space. + const trimmed = event.target.value.trim() + if (trimmed !== event.target.value) update(index, 'id', trimmed) + }} /> { - setEditing({ index, text: event.target.value }) - update(index, 'contextWindow', parseContextWindow(event.target.value)) + const text = event.target.value + setEditing(current => new Map(current).set(index, text)) + update(index, 'contextWindow', parseContextWindow(text)) }} onBlur={() => { settleContext(index) }} /> diff --git a/packages/client/ui-models/tests/components.spec.tsx b/packages/client/ui-models/tests/components.spec.tsx index d9ebadd1f3..7346332a2f 100644 --- a/packages/client/ui-models/tests/components.spec.tsx +++ b/packages/client/ui-models/tests/components.spec.tsx @@ -477,6 +477,91 @@ describe('ModelsSection', () => { .toEqual(base === undefined ? ['deepseek-v4-flash', 'deepseek-v4-pro'] : ['pinned-by-deployment']) }) + it('keeps every row\'s unreadable text, not just the last one edited', async () => { + // The regression: one active buffer meant editing a second row displaced + // the first, which then fell back to rendering its stored NaN as `NaN` — + // losing the text the user was told they could still correct. + await mountSection() + fireEvent.click(screen.getByText(en.customized)) + const windows = screen.getAllByLabelText(new RegExp(en.contextWindow)) + fireEvent.change(windows[0] as HTMLInputElement, { target: { value: 'not a number' } }) + fireEvent.blur(windows[0] as HTMLInputElement) + fireEvent.change(windows[1] as HTMLInputElement, { target: { value: '2M' } }) + + expect((windows[0] as HTMLInputElement).value).toBe('not a number') + expect((windows[1] as HTMLInputElement).value).toBe('2M') + }) + + it('re-keys the typed text around a removed row', async () => { + await mountSection() + fireEvent.click(screen.getByText(en.customized)) + const windows = (): HTMLInputElement[] => + screen.getAllByLabelText(new RegExp(en.contextWindow)) + const removeRow = (at: number): void => { + fireEvent.click(screen.getAllByText(en.removeModel)[at] as HTMLElement) + } + // Three rows, with text parked on the outer two. + fireEvent.click(screen.getByText(en.addModel)) + fireEvent.change(windows()[0] as HTMLInputElement, { target: { value: 'top text' } }) + fireEvent.blur(windows()[0] as HTMLInputElement) + fireEvent.change(windows()[2] as HTMLInputElement, { target: { value: 'bottom text' } }) + fireEvent.blur(windows()[2] as HTMLInputElement) + + // Dropping the middle row leaves the row above untouched and carries the + // row below down with its own text, rather than stranding it. + removeRow(1) + expect(windows()).toHaveLength(2) + expect((windows()[0] as HTMLInputElement).value).toBe('top text') + expect((windows()[1] as HTMLInputElement).value).toBe('bottom text') + + // Dropping a row that holds text takes that text with it; the survivor + // keeps its own rather than inheriting the deleted row's. + removeRow(0) + expect(windows()).toHaveLength(1) + expect((windows()[0] as HTMLInputElement).value).toBe('bottom text') + }) + + it('drops the typed text when reset replaces the rows it annotated', async () => { + // The regression: reset removed the override but left the buffer, so an + // inherited row displayed text no settings layer stores — and because an + // unreadable buffer never settles, it stayed there indefinitely. + const { mutate } = await mountSection({ + mutate: vi.fn(() => Promise.resolve(ok(wireNamespaces()[0]))), + }) + fireEvent.click(screen.getByText(en.customized)) + const windows = screen.getAllByLabelText(new RegExp(en.contextWindow)) + fireEvent.change(windows[0] as HTMLInputElement, { target: { value: 'garbage' } }) + fireEvent.blur(windows[0] as HTMLInputElement) + fireEvent.click(screen.getByText(en.resetModels)) + + const restored = screen.getAllByLabelText(new RegExp(en.contextWindow)) + expect((restored[0] as HTMLInputElement).value).toBe('1M') + + // Reset put the draft back where it started, so Apply writes nothing at + // all rather than persisting whatever the stale text had parsed to. + fireEvent.click(screen.getByText(en.apply)) + await waitFor(() => { expect(screen.getByText(en.apply)).toBeTruthy() }) + expect(mutate).not.toHaveBeenCalled() + }) + + it('settles a pasted id and refuses whitespace that would never match', async () => { + await mountSection() + fireEvent.click(screen.getByText(en.customized)) + const ids = screen.getAllByLabelText(new RegExp(en.modelId)) + fireEvent.change(ids[0] as HTMLInputElement, { target: { value: ' deepseek-v4-flash ' } }) + fireEvent.blur(ids[0] as HTMLInputElement) + expect((ids[0] as HTMLInputElement).value).toBe('deepseek-v4-flash') + // A settled id needs no second trim. + fireEvent.blur(ids[0] as HTMLInputElement) + expect((ids[0] as HTMLInputElement).value).toBe('deepseek-v4-flash') + + // An id that is only whitespace is as absent as an empty one, and a padded + // id no longer slips past the duplicate check against its own twin. + expect(validateDeepSeekModels([{ id: ' ' }])).toEqual({ index: 0, key: 'modelIdRequired' }) + expect(validateDeepSeekModels([{ id: 'model' }, { id: 'model ' }])) + .toEqual({ index: 1, key: 'modelIdDuplicate' }) + }) + it('renders malformed draft fallbacks without inventing catalog values', () => { render(