mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
perf(mcp): stop advertising placeholder input descriptions (#1479)
Input schema presentation gave every undescribed property a generic "Value for <name>." sentence. That only restates the property name, yet it covered 1,958 properties: 70 KB of the 522 KB of advertised input schemas that clients load into model context. Authored descriptions, referenced descriptions, and the name-pattern sentences that add meaning (maximums, digests, paths, Evidence IDs) remain; other properties now advertise no description. The schema validity test now rejects placeholder descriptions instead of requiring a description on every property. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
5f6bb7a5ba
commit
9defed9725
4 files changed
+31
-22
No files matched your search
@@ -59,7 +59,7 @@ const referencedDescription = (
|
||||
const describeSchema = (
|
||||
schema: unknown,
|
||||
root: Readonly<Record<string, unknown>>,
|
||||
describeProperty: (property: string) => string,
|
||||
describeProperty: (property: string) => string | undefined,
|
||||
): unknown => {
|
||||
if (!isObject(schema)) return schema;
|
||||
return Object.fromEntries(
|
||||
@@ -70,16 +70,16 @@ const describeSchema = (
|
||||
Object.fromEntries(
|
||||
Object.entries(value).map(([name, child]) => {
|
||||
const described = describeSchema(child, root, describeProperty);
|
||||
if (key !== "properties" || !isObject(described))
|
||||
return [name, described];
|
||||
const description =
|
||||
referencedDescription(root, described) ??
|
||||
describeProperty(name);
|
||||
return [
|
||||
name,
|
||||
key === "properties" && isObject(described)
|
||||
? {
|
||||
...described,
|
||||
description:
|
||||
referencedDescription(root, described) ??
|
||||
describeProperty(name),
|
||||
}
|
||||
: described,
|
||||
description === undefined
|
||||
? described
|
||||
: { ...described, description },
|
||||
];
|
||||
}),
|
||||
),
|
||||
@@ -223,7 +223,7 @@ const flattenRootObjectUnion = (
|
||||
/** Describe canonical input fields and present the root as one object. */
|
||||
export const presentInputJsonSchema = (
|
||||
root: Record<string, unknown>,
|
||||
describeProperty: (property: string) => string,
|
||||
describeProperty: (property: string) => string | undefined,
|
||||
): Record<string, unknown> => {
|
||||
const flattened = flattenRootObjectUnion(root);
|
||||
const described = describeSchema(flattened, flattened, describeProperty);
|
||||
|
||||
@@ -97,9 +97,14 @@ describe("advertised input references", () => {
|
||||
before: { description: "Exact byte range in the selected artifact." },
|
||||
after: { description: "Exact byte range in the selected artifact." },
|
||||
metadata: { default: literal, examples: [literal] },
|
||||
constructor: { description: "Value for constructor." },
|
||||
},
|
||||
});
|
||||
// Inherited object keys never supply a description.
|
||||
expect(inline).not.toHaveProperty([
|
||||
"properties",
|
||||
"constructor",
|
||||
"description",
|
||||
]);
|
||||
const ajv =
|
||||
target === "draft-2020-12"
|
||||
? new Ajv2020({ strict: false })
|
||||
|
||||
@@ -164,7 +164,7 @@ const memoizeByTarget = (
|
||||
};
|
||||
};
|
||||
|
||||
const fallbackPropertyDescription = (property: string): string => {
|
||||
const fallbackPropertyDescription = (property: string): string | undefined => {
|
||||
const explicit = Object.hasOwn(PROPERTY_DESCRIPTIONS, property)
|
||||
? PROPERTY_DESCRIPTIONS[property]
|
||||
: undefined;
|
||||
@@ -192,5 +192,6 @@ const fallbackPropertyDescription = (property: string): string => {
|
||||
return `Local filesystem ${words} selected for this operation.`;
|
||||
if (property.startsWith("is_") || property.startsWith("has_"))
|
||||
return `Whether ${words}.`;
|
||||
return `Value for ${words}.`;
|
||||
// A generic sentence would only restate the property name in every schema.
|
||||
return undefined;
|
||||
};
|
||||
@@ -117,7 +117,7 @@ function expectKnownAuthorityHints(tools: readonly ToolSchemas[]): void {
|
||||
expect(advertised.get(name), name).toMatchObject({ annotations });
|
||||
}
|
||||
|
||||
function expectRecursivePropertyDescriptions(
|
||||
function expectMeaningfulPropertyDescriptions(
|
||||
schema: unknown,
|
||||
path: string,
|
||||
root: unknown = schema,
|
||||
@@ -125,7 +125,7 @@ function expectRecursivePropertyDescriptions(
|
||||
): void {
|
||||
if (!isRecord(schema)) return;
|
||||
if (typeof schema.$ref === "string" && !active.has(schema.$ref))
|
||||
expectRecursivePropertyDescriptions(
|
||||
expectMeaningfulPropertyDescriptions(
|
||||
resolveReference(root, schema.$ref),
|
||||
`${path}.${schema.$ref}`,
|
||||
root,
|
||||
@@ -133,10 +133,13 @@ function expectRecursivePropertyDescriptions(
|
||||
);
|
||||
if (isRecord(schema.properties)) {
|
||||
for (const [property, child] of Object.entries(schema.properties)) {
|
||||
expect(child, `${path}.${property}`).toMatchObject({
|
||||
description: expect.any(String),
|
||||
});
|
||||
expectRecursivePropertyDescriptions(
|
||||
// Omit a description rather than restate the property name.
|
||||
const description = isRecord(child) ? child.description : undefined;
|
||||
if (description !== undefined)
|
||||
expect(description, `${path}.${property}`).toEqual(
|
||||
expect.not.stringMatching(/^(?:Value for .*)?$/u),
|
||||
);
|
||||
expectMeaningfulPropertyDescriptions(
|
||||
child,
|
||||
`${path}.${property}`,
|
||||
root,
|
||||
@@ -147,12 +150,12 @@ function expectRecursivePropertyDescriptions(
|
||||
|
||||
for (const key of ["items", "additionalProperties"])
|
||||
if (schema[key] !== undefined)
|
||||
expectRecursivePropertyDescriptions(schema[key], path, root, active);
|
||||
expectMeaningfulPropertyDescriptions(schema[key], path, root, active);
|
||||
for (const key of ["allOf", "anyOf", "oneOf", "prefixItems"]) {
|
||||
const children = schema[key];
|
||||
if (Array.isArray(children))
|
||||
children.forEach((child: unknown, index: number) =>
|
||||
expectRecursivePropertyDescriptions(
|
||||
expectMeaningfulPropertyDescriptions(
|
||||
child,
|
||||
`${path}.${key}[${index}]`,
|
||||
root,
|
||||
@@ -358,7 +361,7 @@ describe("MCP JSON Schema validity", () => {
|
||||
contract.inputSchema.safeParse(example.input).success,
|
||||
`${contract.name}: ${example.title}`,
|
||||
).toBe(true);
|
||||
expectRecursivePropertyDescriptions(tool?.inputSchema, contract.name);
|
||||
expectMeaningfulPropertyDescriptions(tool?.inputSchema, contract.name);
|
||||
}
|
||||
} finally {
|
||||
await Promise.allSettled([client.close(), server.close()]);
|
||||
|
||||
Reference in new issue
Block a user