Skip to content

Commit ed9e4dd

Browse files
yulunzyulunz
authored
chore: make tool_call_metrics.json append only against new changes. (#1778)
With this change new tools are added to the very back of the json file. Any removed tools will receive a isDeprecated flag in the existing entry. And the same with tool arguments.
1 parent bc8ef4b commit ed9e4dd

File tree

3 files changed

+251
-7
lines changed

3 files changed

+251
-7
lines changed

‎scripts/update_tool_call_metrics.ts‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import * as fs from 'node:fs';
88
import * as path from 'node:path';
99

1010
import type {ParsedArguments} from '../build/src/bin/chrome-devtools-mcp-cli-options.js';
11-
import {generateToolMetrics} from '../build/src/telemetry/toolMetricsUtils.js';
11+
import {
12+
applyToExistingMetrics,
13+
generateToolMetrics,
14+
type ToolMetric,
15+
} from '../build/src/telemetry/toolMetricsUtils.js';
1216
import type {ToolDefinition} from '../build/src/tools/ToolDefinition.js';
1317
import {createTools} from '../build/src/tools/tools.js';
1418

@@ -35,16 +39,26 @@ function writeToolCallMetricsConfig() {
3539
throw new Error('Error: Duplicate tool names found.');
3640
}
3741

38-
// Map tools to their metadata
39-
const toolData = generateToolMetrics(allTools);
42+
let existingMetrics: ToolMetric[] = [];
43+
if (fs.existsSync(outputPath)) {
44+
try {
45+
existingMetrics = JSON.parse(
46+
fs.readFileSync(outputPath, 'utf8'),
47+
) as ToolMetric[];
48+
} catch {
49+
console.warn(
50+
`Warning: Failed to parse existing metrics from ${outputPath}. Starting fresh.`,
51+
);
52+
}
53+
}
4054

41-
// Sort by name for determinism
42-
toolData.sort((a, b) => a.name.localeCompare(b.name));
55+
const newMetrics = generateToolMetrics(allTools);
56+
const mergedMetrics = applyToExistingMetrics(existingMetrics, newMetrics);
4357

44-
fs.writeFileSync(outputPath, JSON.stringify(toolData, null, 2) + '\n');
58+
fs.writeFileSync(outputPath, JSON.stringify(mergedMetrics, null, 2) + '\n');
4559

4660
console.log(
47-
`Successfully wrote ${toolData.length} tool names with arguments to ${outputPath}`,
61+
`Successfully wrote ${mergedMetrics.length} total tool metrics (including deprecated ones) to ${outputPath}`,
4862
);
4963
}
5064

‎src/telemetry/toolMetricsUtils.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,61 @@ export function validateEnumHomogeneity(values: unknown[]): string {
3030
export interface ArgMetric {
3131
name: string;
3232
argType: string;
33+
isDeprecated?: boolean;
3334
}
3435

3536
export interface ToolMetric {
3637
name: string;
3738
args: ArgMetric[];
39+
isDeprecated?: boolean;
40+
}
41+
42+
export function applyToExistingMetrics(
43+
existing: ToolMetric[],
44+
update: ToolMetric[],
45+
): ToolMetric[] {
46+
const updated = applyToExisting<ToolMetric>(existing, update);
47+
const existingByName = new Map(existing.map(tool => [tool.name, tool]));
48+
const updatedByName = new Map(update.map(tool => [tool.name, tool]));
49+
50+
return updated.map(tool => {
51+
const existingTool = existingByName.get(tool.name);
52+
const updatedTool = updatedByName.get(tool.name);
53+
// If the tool still exists in the update, we will update the args.
54+
if (existingTool && updatedTool) {
55+
const updatedArgs = applyToExisting<ArgMetric>(
56+
existingTool.args,
57+
updatedTool.args,
58+
);
59+
return {...tool, args: updatedArgs};
60+
}
61+
return tool;
62+
});
63+
}
64+
65+
function applyToExisting<T extends {name: string; isDeprecated?: boolean}>(
66+
existing: T[],
67+
update: T[],
68+
): T[] {
69+
const existingNames = new Set(existing.map(item => item.name));
70+
const updatedNames = new Set(update.map(item => item.name));
71+
72+
const result: T[] = [];
73+
// Keep the original ordering.
74+
for (const entry of existing) {
75+
const toAdd = {...entry};
76+
if (!updatedNames.has(entry.name)) {
77+
toAdd.isDeprecated = true;
78+
}
79+
result.push(toAdd);
80+
}
81+
// New entries must be added to the very back of the list.
82+
for (const entry of update) {
83+
if (!existingNames.has(entry.name)) {
84+
result.push({...entry});
85+
}
86+
}
87+
return result;
3888
}
3989

4090
/**

‎tests/telemetry/toolMetricsUtils.test.ts‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import assert from 'node:assert';
88
import {describe, it} from 'node:test';
99

1010
import {
11+
applyToExistingMetrics,
1112
generateToolMetrics,
1213
validateEnumHomogeneity,
1314
} from '../../src/telemetry/toolMetricsUtils.js';
@@ -80,4 +81,183 @@ describe('toolMetricsUtils', () => {
8081
assert.strictEqual(metrics[0].args[0].argType, 'string');
8182
});
8283
});
84+
85+
describe('applyToExistingMetrics', () => {
86+
it('should return the same metrics if existing and update are the same', () => {
87+
const existing = [{name: 'foo', args: []}];
88+
const update = [{name: 'foo', args: []}];
89+
const result = applyToExistingMetrics(existing, update);
90+
const expected = [{name: 'foo', args: []}];
91+
assert.deepStrictEqual(result, expected);
92+
});
93+
94+
it('should append new entries to the end of the array', () => {
95+
const existing = [{name: 'foo', args: []}];
96+
const update = [
97+
{name: 'foo', args: []},
98+
{name: 'bar', args: []},
99+
];
100+
const result = applyToExistingMetrics(existing, update);
101+
const expected = [
102+
{name: 'foo', args: []},
103+
{name: 'bar', args: []},
104+
];
105+
assert.deepStrictEqual(result, expected);
106+
});
107+
108+
it('should mark missing entries as deprecated and preserve their order', () => {
109+
const existing = [
110+
{name: 'foo', args: []},
111+
{name: 'bar', args: []},
112+
];
113+
const update = [{name: 'foo', args: []}];
114+
const result = applyToExistingMetrics(existing, update);
115+
const expected = [
116+
{name: 'foo', args: []},
117+
{name: 'bar', args: [], isDeprecated: true},
118+
];
119+
assert.deepStrictEqual(result, expected);
120+
});
121+
122+
it('should handle adding new entries and deprecating old ones simultaneously', () => {
123+
const existing = [
124+
{name: 'foo', args: []},
125+
{name: 'bar', args: []},
126+
];
127+
const update = [
128+
{name: 'bar', args: []},
129+
{name: 'baz', args: []},
130+
];
131+
const result = applyToExistingMetrics(existing, update);
132+
const expected = [
133+
{name: 'foo', args: [], isDeprecated: true},
134+
{name: 'bar', args: []},
135+
{name: 'baz', args: []},
136+
];
137+
assert.deepStrictEqual(result, expected);
138+
});
139+
140+
it('should append new arguments to the back', () => {
141+
const existing = [
142+
{name: 'foo', args: [{name: 'arg_a', argType: 'string'}]},
143+
];
144+
const update = [
145+
{
146+
name: 'foo',
147+
args: [
148+
{name: 'arg_a', argType: 'string'},
149+
{name: 'arg_b', argType: 'string'},
150+
],
151+
},
152+
];
153+
const result = applyToExistingMetrics(existing, update);
154+
const expected = [
155+
{
156+
name: 'foo',
157+
args: [
158+
{name: 'arg_a', argType: 'string'},
159+
{name: 'arg_b', argType: 'string'},
160+
],
161+
},
162+
];
163+
assert.deepStrictEqual(result, expected);
164+
});
165+
166+
it('should mark removed arguments as deprecated', () => {
167+
const existing = [
168+
{
169+
name: 'foo',
170+
args: [
171+
{name: 'arg_a', argType: 'string'},
172+
{name: 'arg_b', argType: 'string'},
173+
],
174+
},
175+
];
176+
const update = [
177+
{name: 'foo', args: [{name: 'arg_a', argType: 'string'}]},
178+
];
179+
const result = applyToExistingMetrics(existing, update);
180+
const expected = [
181+
{
182+
name: 'foo',
183+
args: [
184+
{name: 'arg_a', argType: 'string'},
185+
{name: 'arg_b', argType: 'string', isDeprecated: true},
186+
],
187+
},
188+
];
189+
assert.deepStrictEqual(result, expected);
190+
});
191+
192+
it('should not change args if they are the same', () => {
193+
const existing = [
194+
{name: 'foo', args: [{name: 'arg_a', argType: 'string'}]},
195+
];
196+
const update = [
197+
{name: 'foo', args: [{name: 'arg_a', argType: 'string'}]},
198+
];
199+
const result = applyToExistingMetrics(existing, update);
200+
const expected = [
201+
{name: 'foo', args: [{name: 'arg_a', argType: 'string'}]},
202+
];
203+
assert.deepStrictEqual(result, expected);
204+
});
205+
206+
it('should handle adding and removing arguments simultaneously', () => {
207+
const existing = [
208+
{
209+
name: 'foo',
210+
args: [
211+
{name: 'arg_a', argType: 'string'},
212+
{name: 'arg_b', argType: 'string'},
213+
],
214+
},
215+
];
216+
const update = [
217+
{
218+
name: 'foo',
219+
args: [
220+
{name: 'arg_b', argType: 'string'},
221+
{name: 'arg_c', argType: 'string'},
222+
],
223+
},
224+
];
225+
const result = applyToExistingMetrics(existing, update);
226+
const expected = [
227+
{
228+
name: 'foo',
229+
args: [
230+
{name: 'arg_a', argType: 'string', isDeprecated: true},
231+
{name: 'arg_b', argType: 'string'},
232+
{name: 'arg_c', argType: 'string'},
233+
],
234+
},
235+
];
236+
assert.deepStrictEqual(result, expected);
237+
});
238+
239+
it('should handle tool and argument changes simultaneously', () => {
240+
const existing = [
241+
{name: 'foo', args: [{name: 'arg_a', argType: 'string'}]},
242+
{name: 'bar', args: []},
243+
];
244+
const update = [
245+
{name: 'foo', args: [{name: 'arg_b', argType: 'string'}]},
246+
{name: 'baz', args: []},
247+
];
248+
const result = applyToExistingMetrics(existing, update);
249+
const expected = [
250+
{
251+
name: 'foo',
252+
args: [
253+
{name: 'arg_a', argType: 'string', isDeprecated: true},
254+
{name: 'arg_b', argType: 'string'},
255+
],
256+
},
257+
{name: 'bar', args: [], isDeprecated: true},
258+
{name: 'baz', args: []},
259+
];
260+
assert.deepStrictEqual(result, expected);
261+
});
262+
});
83263
});

0 commit comments

Comments
 (0)