Skip to content

Commit 8a44929

Browse files
authored
🐛 fix: Preserve typed values when adding entries to new sections (#37)
When the admin panel adds an indexed-array edit (`modelSpecs.list.0`, `endpoints.azureOpenAI.groups.0`) and the array's parent path is absent from the YAML baseline, the merge that builds `activeConfigValues` silently bails out and writes the array at the wrong nesting level. The form's view of the section ends up empty, so typed entries appear to vanish on blur and saves persist `[{}]` instead of the typed values. The fix auto-creates missing intermediate object nodes during the merge. Existing primitive/array values at intermediate paths still bail out so live baseline data is never overwritten. Extracts the merge into a pure `mergeIndexedArrayEdits` helper in `utils.ts` and locks the behavior with nine unit tests covering the regression case, sibling preservation, multi-edit indexing, defensive bail-outs, and deep parent chains. Linear: AI-894
1 parent c6ec479 commit 8a44929

3 files changed

Lines changed: 144 additions & 20 deletions

File tree

src/components/configuration/ConfigPage.tsx

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { StickyActionBar } from '@/components/shared';
3535
import { ConfigTabContent } from './ConfigTabContent';
3636
import { ImportYamlDialog } from './ImportYamlDialog';
3737
import { ContentToolbar } from './ContentToolbar';
38+
import { mergeIndexedArrayEdits } from './utils';
3839
import { SystemCapabilities } from '@/constants';
3940
import { ConfigTabBar } from './ConfigTabBar';
4041
import { InfoBanner } from './InfoBanner';
@@ -276,25 +277,7 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi
276277
if (!baseActiveConfigValues) return baseActiveConfigValues;
277278
const indexedEdits = Object.entries(editedValues).filter(([k]) => /\.\d+$/.test(k));
278279
if (indexedEdits.length === 0) return baseActiveConfigValues;
279-
const merged = { ...baseActiveConfigValues };
280-
for (const [path, value] of indexedEdits) {
281-
const segments = path.split('.');
282-
const index = Number(segments.pop()!);
283-
const arrayPath = segments;
284-
let parent: Record<string, t.ConfigValue> = merged;
285-
for (let i = 0; i < arrayPath.length - 1; i++) {
286-
const seg = arrayPath[i];
287-
if (parent[seg] && typeof parent[seg] === 'object' && !Array.isArray(parent[seg])) {
288-
parent[seg] = { ...(parent[seg] as Record<string, t.ConfigValue>) };
289-
parent = parent[seg] as Record<string, t.ConfigValue>;
290-
} else break;
291-
}
292-
const lastSeg = arrayPath[arrayPath.length - 1];
293-
const arr = Array.isArray(parent[lastSeg]) ? [...(parent[lastSeg] as t.ConfigValue[])] : [];
294-
arr[index] = value;
295-
parent[lastSeg] = arr;
296-
}
297-
return merged;
280+
return mergeIndexedArrayEdits(baseActiveConfigValues, indexedEdits);
298281
}, [baseActiveConfigValues, editedValues]);
299282

300283
const scopeConfiguredPaths = useMemo(() => {

src/components/configuration/utils.test.ts

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { describe, it, expect } from 'vitest';
2-
import { getControlType, getEnumOptions, getArrayItemType, splitUnionTypes } from './utils';
2+
import {
3+
getControlType,
4+
getEnumOptions,
5+
getArrayItemType,
6+
splitUnionTypes,
7+
mergeIndexedArrayEdits,
8+
} from './utils';
39
import { createField } from '@/test/fixtures';
410

511
describe('getControlType', () => {
@@ -236,3 +242,93 @@ describe('getEnumOptions — union(literal(...)) parsing', () => {
236242
expect(getEnumOptions('union(string | number)')).toEqual([]);
237243
});
238244
});
245+
246+
describe('mergeIndexedArrayEdits', () => {
247+
it('creates the array under a parent path absent from the baseline', () => {
248+
/**
249+
* Regression: the merge previously bailed out and wrote the array at the
250+
* wrong nesting level when its parent (e.g. modelSpecs) wasn't in
251+
* librechat.yaml, causing typed list entries to disappear from view.
252+
*/
253+
const merged = mergeIndexedArrayEdits({}, [
254+
['modelSpecs.list.0', { name: 'test', label: 'Test' }],
255+
]);
256+
expect(merged).toEqual({
257+
modelSpecs: { list: [{ name: 'test', label: 'Test' }] },
258+
});
259+
});
260+
261+
it('preserves baseline siblings when introducing a new section', () => {
262+
const merged = mergeIndexedArrayEdits({ interface: { parameters: true } }, [
263+
['modelSpecs.list.0', { name: 'a' }],
264+
]);
265+
expect(merged).toEqual({
266+
interface: { parameters: true },
267+
modelSpecs: { list: [{ name: 'a' }] },
268+
});
269+
});
270+
271+
it('merges into an existing parent without clobbering its keys', () => {
272+
const merged = mergeIndexedArrayEdits(
273+
{ modelSpecs: { enforce: true, prioritize: false } },
274+
[['modelSpecs.list.0', { name: 'a' }]],
275+
);
276+
expect(merged.modelSpecs).toEqual({
277+
enforce: true,
278+
prioritize: false,
279+
list: [{ name: 'a' }],
280+
});
281+
});
282+
283+
it('places multiple indexed edits at their correct positions', () => {
284+
const merged = mergeIndexedArrayEdits({}, [
285+
['modelSpecs.list.0', { name: 'a' }],
286+
['modelSpecs.list.2', { name: 'c' }],
287+
]);
288+
const list = (merged.modelSpecs as { list: Array<{ name: string } | undefined> }).list;
289+
expect(list[0]).toEqual({ name: 'a' });
290+
expect(list[1]).toBeUndefined();
291+
expect(list[2]).toEqual({ name: 'c' });
292+
});
293+
294+
it('returns the baseline unchanged when there are no indexed edits', () => {
295+
const baseline = { interface: { parameters: true } };
296+
expect(mergeIndexedArrayEdits(baseline, [])).toEqual(baseline);
297+
});
298+
299+
it('does not mutate the baseline object', () => {
300+
const baseline: Record<string, unknown> = { modelSpecs: { enforce: true } };
301+
const before = JSON.parse(JSON.stringify(baseline));
302+
mergeIndexedArrayEdits(baseline as Record<string, never>, [
303+
['modelSpecs.list.0', { name: 'a' }],
304+
]);
305+
expect(baseline).toEqual(before);
306+
});
307+
308+
it('skips an edit when an intermediate path is a primitive', () => {
309+
/**
310+
* Defensive: refuse to overwrite a primitive at an intermediate path
311+
* because doing so would silently destroy unrelated baseline data.
312+
*/
313+
const merged = mergeIndexedArrayEdits({ modelSpecs: 'not-an-object' }, [
314+
['modelSpecs.list.0', { name: 'a' }],
315+
]);
316+
expect(merged).toEqual({ modelSpecs: 'not-an-object' });
317+
});
318+
319+
it('skips an edit when an intermediate path is an array', () => {
320+
const merged = mergeIndexedArrayEdits({ modelSpecs: [1, 2, 3] }, [
321+
['modelSpecs.list.0', { name: 'a' }],
322+
]);
323+
expect(merged).toEqual({ modelSpecs: [1, 2, 3] });
324+
});
325+
326+
it('walks deep parent chains, creating each missing level', () => {
327+
const merged = mergeIndexedArrayEdits({}, [
328+
['endpoints.custom.deep.list.0', { name: 'x' }],
329+
]);
330+
expect(merged).toEqual({
331+
endpoints: { custom: { deep: { list: [{ name: 'x' }] } } },
332+
});
333+
});
334+
});

src/components/configuration/utils.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,3 +222,48 @@ export function hasDescendant(path: string, paths?: Set<string>): boolean {
222222
}
223223
return false;
224224
}
225+
226+
/**
227+
* Merge indexed-array edits (entries whose flat path ends in `.<digit>`) into
228+
* a config tree. Each indexed edit's value replaces the array element at that
229+
* index; the array's parent path is auto-created if absent so newly-introduced
230+
* sections (e.g. `modelSpecs` not present in `librechat.yaml`) merge in at the
231+
* correct nesting level rather than getting written to the wrong parent.
232+
*
233+
* Skips an edit when an intermediate path holds a primitive or array value,
234+
* since overwriting those with a fresh object would silently destroy live
235+
* baseline data. Caller is responsible for filtering `editedValues` down to
236+
* indexed entries before passing.
237+
*/
238+
export function mergeIndexedArrayEdits(
239+
baseline: Record<string, t.ConfigValue>,
240+
indexedEdits: Array<[string, t.ConfigValue]>,
241+
): Record<string, t.ConfigValue> {
242+
const merged = { ...baseline };
243+
for (const [path, value] of indexedEdits) {
244+
const segments = path.split('.');
245+
const index = Number(segments.pop()!);
246+
const arrayPath = segments;
247+
let parent: Record<string, t.ConfigValue> = merged;
248+
let bailed = false;
249+
for (let i = 0; i < arrayPath.length - 1; i++) {
250+
const seg = arrayPath[i];
251+
const existing = parent[seg];
252+
if (existing && typeof existing === 'object' && !Array.isArray(existing)) {
253+
parent[seg] = { ...(existing as Record<string, t.ConfigValue>) };
254+
} else if (existing == null) {
255+
parent[seg] = {};
256+
} else {
257+
bailed = true;
258+
break;
259+
}
260+
parent = parent[seg] as Record<string, t.ConfigValue>;
261+
}
262+
if (bailed) continue;
263+
const lastSeg = arrayPath[arrayPath.length - 1];
264+
const arr = Array.isArray(parent[lastSeg]) ? [...(parent[lastSeg] as t.ConfigValue[])] : [];
265+
arr[index] = value;
266+
parent[lastSeg] = arr;
267+
}
268+
return merged;
269+
}

0 commit comments

Comments
 (0)