From 939b31bbbd83cfe79279f3cf288d3c61cd529a20 Mon Sep 17 00:00:00 2001 From: Nikhil Bhagat Date: Sun, 18 Oct 2026 12:23:29 +0545 Subject: [PATCH] fix(setup): delete repeated JSON members in one pass Deleting repeats one at a time still copied the whole document once per member, so tens of thousands of repeats in a large file cost k x n work. All removal spans now come from one syntax tree and the text is rebuilt once. A separator stays only between members that remain, plus a JSONC trailing separator while any member remains, and comments outside the removed members keep their bytes. The single-member deletion from #4957 is unchanged. Removing 20,000 repeats drops from 35.8 s to 0.46 s. Co-Authored-By: Claude Opus 5.5 --- ...igurationDocument.duplicateMembers.test.ts | 8 +-- .../ClientConfigurationDocument.ts | 50 +++++++++++++------ 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/application/ClientConfigurationDocument.duplicateMembers.test.ts b/src/application/ClientConfigurationDocument.duplicateMembers.test.ts index fe3ac08b77..dfc313308a 100644 --- a/src/application/ClientConfigurationDocument.duplicateMembers.test.ts +++ b/src/application/ClientConfigurationDocument.duplicateMembers.test.ts @@ -229,10 +229,10 @@ describe("JSON client configuration with repeated members", () => { }); describe("JSON client configuration with many repeated members", () => { - it("removes many repeats from one syntax tree", () => { + it("removes tens of thousands of repeats in one pass", () => { // Repeats alternate with peers and comments, so every separator case occurs. const members = Array.from( - { length: 2_000 }, + { length: 20_000 }, (_, index) => `"rea": ${String(index)}, /* ${String(index)} */ "p${String(index)}": ${String(index)}`, ); @@ -241,11 +241,11 @@ describe("JSON client configuration with many repeated members", () => { removeShadowedMembers: true, }); if (!result.ok) throw new Error("removal refused"); - expect(Object.keys(remaining)).toHaveLength(2_000); + expect(Object.keys(remaining)).toHaveLength(20_000); expect(parseClientConfiguration(result.value, "json").document).toEqual({ mcpServers: remaining, }); expect(result.value).not.toContain('"rea"'); - expect(result.value).toContain("/* 1999 */"); + expect(result.value).toContain("/* 19999 */"); }); }); diff --git a/src/application/ClientConfigurationDocument.ts b/src/application/ClientConfigurationDocument.ts index a0b11e9f72..cf091cc6e9 100644 --- a/src/application/ClientConfigurationDocument.ts +++ b/src/application/ClientConfigurationDocument.ts @@ -1471,9 +1471,10 @@ const applyFormattedJsonEdit = (text: string, edit: Edit): string => { */ const deleteJsonProperty = ( text: string, - siblings: readonly Node[], + object: Node, property: Node, ): string => { + const siblings = object.children ?? []; const propertyIndex = siblings.indexOf(property); const previous = siblings[propertyIndex - 1]; const next = siblings[propertyIndex + 1]; @@ -1506,26 +1507,45 @@ export interface ClientConfigurationShadowedMember { } /** - * Delete every member named `key` from `object`, parsed from `text`. Removing - * only the effective member would make the previous repeat effective. Members - * are deleted from the last one back: each deletion edits text at or after the - * end of the member before it, so the earlier offsets in this one syntax tree - * stay valid, and separators are judged against the members still present. + * Delete every member named `key` from `object`, parsed from `text`, in one + * pass. Removing only the effective member would make the previous repeat + * effective. A separator stays only between members that remain, plus a JSONC + * trailing separator while any member remains; comments and whitespace + * outside the removed members and separators keep their bytes. */ const deleteRepeatedJsonMember = ( text: string, object: Node, key: string, ): string => { - const siblings = [...(object.children ?? [])]; - let edited = text; - for (const property of siblings - .filter((member) => isJsonMember(member, key)) - .reverse()) { - edited = deleteJsonProperty(edited, siblings, property); - siblings.splice(siblings.indexOf(property), 1); + const siblings = object.children ?? []; + const removed = siblings.map((member) => isJsonMember(member, key)); + const anyKept = removed.includes(false); + const spans: { readonly start: number; readonly end: number }[] = []; + // Walk back so `laterKept` says whether a kept member follows a separator. + let laterKept = false; + for (const [index, member] of [...siblings.entries()].reverse()) { + const end = member.offset + member.length; + const separator = nextJsoncToken(text, end); + const last = index === siblings.length - 1; + // Admitted documents always separate members, so this is a parser defect. + if (separator.kind !== JSON_COMMA && !last) + throw new TypeError("JSON property separator was not found"); + const keepSeparator = last ? anyKept : removed[index] !== true && laterKept; + if (separator.kind === JSON_COMMA && !keepSeparator) + spans.push({ start: separator.offset, end: separator.offset + 1 }); + if (removed[index] === true) spans.push({ start: member.offset, end }); + else laterKept = true; } - return edited; + // Spans were collected back to front and never overlap. + const kept: string[] = []; + let cursor = 0; + for (const span of spans.reverse()) { + kept.push(text.slice(cursor, span.start)); + cursor = span.end; + } + kept.push(text.slice(cursor)); + return kept.join(""); }; /** @@ -1588,7 +1608,7 @@ const editJsonMember = ( return removeShadowedMembers ? ok(deleteRepeatedJsonMember(text, object, key)) : err({ kind: "shadowed_member", path }); - return ok(deleteJsonProperty(text, object.children ?? [], property)); + return ok(deleteJsonProperty(text, object, property)); } throw new TypeError("JSON edit path is empty"); };