Skip to content

Commit 01bd98c

Browse files
authored
refactor(mcp): shared trim util + global response size-budget net (open-metadata#28764)
* refactor(mcp): shared response-trim + params utils, global size-budget net, error-message null-guards * refactor(mcp): expose serializeWithinBudget so Collate dispatcher shares the size-budget net * refactor(mcp): move McpParams/McpResponseTrim to util package, guard RCA error messages
1 parent 4699608 commit 01bd98c

11 files changed

Lines changed: 510 additions & 205 deletions

File tree

openmetadata-mcp/src/main/java/org/openmetadata/mcp/tools/DefaultToolContext.java

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.util.Set;
1212
import java.util.function.Predicate;
1313
import lombok.extern.slf4j.Slf4j;
14+
import org.openmetadata.mcp.util.McpResponseTrim;
1415
import org.openmetadata.schema.entity.app.mcp.McpToolCallUsage;
1516
import org.openmetadata.schema.utils.JsonUtils;
1617
import org.openmetadata.service.limits.Limits;
@@ -26,6 +27,9 @@ public class DefaultToolContext {
2627
private static final int STATUS_TOO_MANY_REQUESTS = 429;
2728
private static final int STATUS_INTERNAL_ERROR = 500;
2829
private static final int STATUS_GATEWAY_TIMEOUT = 504;
30+
private static final String OVERSIZED_ADVICE =
31+
"Response exceeded the size limit and was withheld. Narrow your request — use a more specific "
32+
+ "query, request fewer results, or fetch a single entity by its fullyQualifiedName.";
2933

3034
public DefaultToolContext() {}
3135

@@ -145,7 +149,7 @@ public CallToolOutcome callToolWithMetadata(
145149

146150
return new CallToolOutcome(
147151
McpSchema.CallToolResult.builder()
148-
.content(List.of(new McpSchema.TextContent(JsonUtils.pojoToJson(result))))
152+
.content(List.of(new McpSchema.TextContent(serializeWithinBudget(result, toolName))))
149153
.isError(false)
150154
.build(),
151155
elapsedMs(startNanos),
@@ -160,7 +164,8 @@ public CallToolOutcome callToolWithMetadata(
160164
JsonUtils.pojoToJson(
161165
Map.of(
162166
"error",
163-
String.format("Authorization error: %s", ex.getMessage()),
167+
String.format(
168+
"Authorization error: %s", McpResponseTrim.safeMessage(ex)),
164169
"statusCode",
165170
STATUS_FORBIDDEN)))))
166171
.isError(true)
@@ -177,7 +182,8 @@ public CallToolOutcome callToolWithMetadata(
177182
JsonUtils.pojoToJson(
178183
Map.of(
179184
"error",
180-
String.format("Error executing tool: %s", ex.getMessage()),
185+
String.format(
186+
"Error executing tool: %s", McpResponseTrim.safeMessage(ex)),
181187
"statusCode",
182188
resolveStatusCode(ex))))))
183189
.isError(true)
@@ -300,6 +306,32 @@ private static long elapsedMs(long startNanos) {
300306
return (System.nanoTime() - startNanos) / 1_000_000L;
301307
}
302308

309+
/**
310+
* Serializes a tool result once and, only when it exceeds {@link
311+
* McpResponseTrim#MAX_RESPONSE_CHARS}, replaces it with a generic {@code truncated:true} envelope.
312+
* This is the dispatch-level floor that bounds tools without their own per-tool trim ({@code
313+
* get_entity_details}, {@code get_test_definitions}) and backstops the rest. The happy path
314+
* serializes exactly once; the re-serialization runs only on the rare oversized path.
315+
*
316+
* <p>Public so the Collate dispatcher ({@code CollateToolContext}), which builds its own success
317+
* result for Collate-only tools, applies the same floor instead of re-implementing it.
318+
*/
319+
public static String serializeWithinBudget(Object result, String toolName) {
320+
String serialized = JsonUtils.pojoToJson(result);
321+
if (serialized.length() > McpResponseTrim.MAX_RESPONSE_CHARS) {
322+
LOG.warn(
323+
"[MCP] tool '{}' response {} chars exceeds {} budget; returning truncation envelope",
324+
toolName,
325+
serialized.length(),
326+
McpResponseTrim.MAX_RESPONSE_CHARS);
327+
Map<String, Object> capped =
328+
McpResponseTrim.oversizedEnvelope(
329+
serialized.length(), Map.of("tool", toolName), OVERSIZED_ADVICE);
330+
serialized = JsonUtils.pojoToJson(capped);
331+
}
332+
return serialized;
333+
}
334+
303335
/**
304336
* Phase 3 — tuple returned by {@link #callToolWithMetadata} so the MCP server can record the
305337
* call with full diagnostic detail without re-classifying the exception or re-measuring the

openmetadata-mcp/src/main/java/org/openmetadata/mcp/tools/GetEntityTool.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.util.List;
88
import java.util.Map;
99
import lombok.extern.slf4j.Slf4j;
10+
import org.openmetadata.mcp.util.McpResponseTrim;
1011
import org.openmetadata.schema.utils.JsonUtils;
1112
import org.openmetadata.service.Entity;
1213
import org.openmetadata.service.limits.Limits;
@@ -42,8 +43,7 @@ public class GetEntityTool implements McpTool {
4243
"tagSources",
4344
"descriptionSources",
4445
"columnDescriptionStatus",
45-
"descriptionStatus",
46-
"embeddings");
46+
"descriptionStatus");
4747

4848
@Override
4949
public Map<String, Object> execute(
@@ -74,6 +74,7 @@ private static Map<String, Object> cleanEntityResponse(Map<String, Object> entit
7474
}
7575
Map<String, Object> cleaned = new HashMap<>(entityData);
7676
EXCLUDE_FIELDS.forEach(cleaned::remove);
77+
McpResponseTrim.VECTOR_NOISE_FIELDS.forEach(cleaned::remove);
7778
return cleaned;
7879
}
7980

openmetadata-mcp/src/main/java/org/openmetadata/mcp/tools/GetLineageTool.java

Lines changed: 15 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import java.util.Set;
1414
import java.util.UUID;
1515
import lombok.extern.slf4j.Slf4j;
16+
import org.openmetadata.mcp.util.McpParams;
17+
import org.openmetadata.mcp.util.McpResponseTrim;
1618
import org.openmetadata.schema.type.ColumnLineage;
1719
import org.openmetadata.schema.type.Edge;
1820
import org.openmetadata.schema.type.EntityLineage;
@@ -43,13 +45,6 @@ public class GetLineageTool implements McpTool {
4345
private static final int DEFAULT_DEPTH = 3;
4446
// Maximum depth to prevent exponential response growth (lineage graphs can explode)
4547
private static final int MAX_DEPTH = 10;
46-
// SQL is the single heaviest field; keep the gist of the transform, cap the size
47-
private static final int SQL_MAX_LENGTH = 500;
48-
// Free-text markdown (pipeline / edge descriptions) is capped for the same reason as SQL:
49-
// a single long pipeline doc string can be shared across many edges and reintroduce bloat
50-
private static final int TEXT_MAX_LENGTH = 500;
51-
// Final safety net mirroring SearchMetadataTool: even slimmed, a wide graph can blow the limit
52-
private static final int MAX_RESPONSE_CHARS = 100_000;
5348
private static final String RELATIONSHIP_SQL = "sql";
5449

5550
@JsonInclude(JsonInclude.Include.NON_NULL)
@@ -94,9 +89,9 @@ public Map<String, Object> execute(
9489
securityContext,
9590
new OperationContext(entityType, MetadataOperation.VIEW_BASIC),
9691
new ResourceContext<>(entityType));
97-
int upstreamDepth = parseDepthParameter(params.get("upstreamDepth"), DEFAULT_DEPTH);
98-
int downstreamDepth = parseDepthParameter(params.get("downstreamDepth"), DEFAULT_DEPTH);
99-
boolean includeColumnLineage = parseBooleanParameter(params.get("includeColumnLineage"));
92+
int upstreamDepth = clampDepth(McpParams.getInt(params, "upstreamDepth", DEFAULT_DEPTH));
93+
int downstreamDepth = clampDepth(McpParams.getInt(params, "downstreamDepth", DEFAULT_DEPTH));
94+
boolean includeColumnLineage = McpParams.getBoolean(params, "includeColumnLineage", false);
10095
LOG.info(
10196
"Getting lineage for entity type: {}, FQN: {}, upstreamDepth: {}, downstreamDepth: {}, "
10297
+ "includeColumnLineage: {}",
@@ -212,11 +207,7 @@ private static String relationshipType(EntityReference pipeline) {
212207
}
213208

214209
private static String truncateText(String text) {
215-
String result = text;
216-
if (text != null && text.length() > TEXT_MAX_LENGTH) {
217-
result = text.substring(0, TEXT_MAX_LENGTH) + "...";
218-
}
219-
return result;
210+
return McpResponseTrim.truncate(text, McpResponseTrim.TEXT_MAX_LENGTH);
220211
}
221212

222213
private static String sourceValue(LineageDetails details) {
@@ -227,8 +218,8 @@ private static SqlText truncateSqlQuery(LineageDetails details) {
227218
String sql = details != null ? details.getSqlQuery() : null;
228219
SqlText result = new SqlText(null, null);
229220
if (sql != null) {
230-
boolean tooLong = sql.length() > SQL_MAX_LENGTH;
231-
String value = tooLong ? sql.substring(0, SQL_MAX_LENGTH) + "..." : sql;
221+
boolean tooLong = sql.length() > McpResponseTrim.SQL_MAX_LENGTH;
222+
String value = McpResponseTrim.truncate(sql, McpResponseTrim.SQL_MAX_LENGTH);
232223
result = new SqlText(value, tooLong ? Boolean.TRUE : null);
233224
}
234225
return result;
@@ -253,9 +244,9 @@ private static String refName(EntityReference ref) {
253244
@VisibleForTesting
254245
static Map<String, Object> enforceSizeBudget(SlimLineage slim) {
255246
Map<String, Object> response = JsonUtils.getMap(slim);
256-
int responseSize = JsonUtils.pojoToJson(response).length();
247+
int responseSize = McpResponseTrim.serializedLength(response);
257248
Map<String, Object> result = response;
258-
if (responseSize > MAX_RESPONSE_CHARS) {
249+
if (responseSize > McpResponseTrim.MAX_RESPONSE_CHARS) {
259250
result = oversizedHint(slim, responseSize);
260251
}
261252
return result;
@@ -276,44 +267,19 @@ private static Map<String, Object> oversizedHint(SlimLineage slim, int size) {
276267
String.format(
277268
"Lineage response exceeded %d characters (was %d). Reduce upstreamDepth/downstreamDepth,"
278269
+ " or keep includeColumnLineage disabled, to get a smaller graph.",
279-
MAX_RESPONSE_CHARS, size));
270+
McpResponseTrim.MAX_RESPONSE_CHARS, size));
280271
return hint;
281272
}
282273

283-
private static boolean parseBooleanParameter(Object value) {
284-
boolean result = false;
285-
if (value instanceof Boolean bool) {
286-
result = bool;
287-
} else if (value instanceof String string) {
288-
result = Boolean.parseBoolean(string);
289-
}
290-
return result;
291-
}
292-
293274
/**
294-
* Parses depth parameter with default value and enforces maximum limit to prevent excessive
295-
* response sizes that could overwhelm LLM context.
275+
* Clamps a requested depth into {@code [1, MAX_DEPTH]} to prevent excessive response sizes that
276+
* could overwhelm LLM context. Parsing is delegated to {@link McpParams}; the valid range is
277+
* specific to this tool, so the clamp stays here.
296278
*/
297-
private static int parseDepthParameter(Object depthObj, int defaultValue) {
298-
int depth = defaultValue;
299-
if (depthObj instanceof Number number) {
300-
depth = number.intValue();
301-
} else if (depthObj instanceof String string) {
302-
depth = parseDepthString(string, defaultValue);
303-
}
279+
private static int clampDepth(int depth) {
304280
return Math.min(Math.max(depth, 1), MAX_DEPTH);
305281
}
306282

307-
private static int parseDepthString(String value, int defaultValue) {
308-
int depth = defaultValue;
309-
try {
310-
depth = Integer.parseInt(value);
311-
} catch (NumberFormatException e) {
312-
depth = defaultValue;
313-
}
314-
return depth;
315-
}
316-
317283
@Override
318284
public Map<String, Object> execute(
319285
Authorizer authorizer,

openmetadata-mcp/src/main/java/org/openmetadata/mcp/tools/RootCauseAnalysisTool.java

Lines changed: 18 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import java.util.Map;
1616
import java.util.Set;
1717
import lombok.extern.slf4j.Slf4j;
18+
import org.openmetadata.mcp.util.McpParams;
19+
import org.openmetadata.mcp.util.McpResponseTrim;
1820
import org.openmetadata.schema.api.lineage.LineageDirection;
1921
import org.openmetadata.schema.api.lineage.SearchLineageRequest;
2022
import org.openmetadata.schema.api.lineage.SearchLineageResult;
@@ -36,13 +38,10 @@ public class RootCauseAnalysisTool implements McpTool {
3638

3739
private static final int DEFAULT_DEPTH = 3;
3840
private static final int MAX_DEPTH = 10;
39-
// Slimming budgets mirror GetLineageTool so RCA's lineage-derived payload stays within
41+
// Slimming budgets come from McpResponseTrim so RCA's lineage-derived payload stays within
4042
// LLM/MCP context limits. The backend (searchDataQualityLineage / searchLineageWithDirection)
4143
// is shared with the UI LineageResource and is never touched — we only transform the
4244
// in-memory result before returning it to the MCP client.
43-
private static final int SQL_MAX_LENGTH = 500;
44-
private static final int TEXT_MAX_LENGTH = 500;
45-
private static final int MAX_RESPONSE_CHARS = 100_000;
4645
private static final String RELATIONSHIP_SQL = "sql";
4746

4847
@Override
@@ -52,12 +51,12 @@ public Map<String, Object> execute(
5251
Map<String, Object> parameters) {
5352
String fqn = (String) parameters.get("fqn");
5453
String entityType = (String) parameters.getOrDefault("entityType", "table");
55-
int upstreamDepth = clampDepth(parseIntParam(parameters.get("upstreamDepth"), DEFAULT_DEPTH));
54+
int upstreamDepth = clampDepth(McpParams.getInt(parameters, "upstreamDepth", DEFAULT_DEPTH));
5655
int downstreamDepth =
57-
clampDepth(parseIntParam(parameters.get("downstreamDepth"), DEFAULT_DEPTH));
56+
clampDepth(McpParams.getInt(parameters, "downstreamDepth", DEFAULT_DEPTH));
5857
String queryFilter = (String) parameters.get("queryFilter");
59-
boolean includeDeleted = parseBooleanParam(parameters.get("includeDeleted"), false);
60-
boolean includeColumns = parseBooleanParam(parameters.get("includeColumnLineage"), false);
58+
boolean includeDeleted = McpParams.getBoolean(parameters, "includeDeleted", false);
59+
boolean includeColumns = McpParams.getBoolean(parameters, "includeColumnLineage", false);
6160

6261
if (fqn == null || fqn.trim().isEmpty()) {
6362
throw new IllegalArgumentException("Parameter 'fqn' is required and cannot be empty");
@@ -81,11 +80,12 @@ public Map<String, Object> execute(
8180
return analyze(request);
8281
} catch (IOException e) {
8382
LOG.error("IOException during root cause analysis for entity: {}", fqn, e);
84-
throw new RuntimeException("Failed to perform root cause analysis: " + e.getMessage(), e);
83+
throw new RuntimeException(
84+
"Failed to perform root cause analysis: " + McpResponseTrim.safeMessage(e), e);
8585
} catch (Exception e) {
8686
LOG.error("Unexpected error during root cause analysis for entity: {}", fqn, e);
8787
throw new RuntimeException(
88-
"Unexpected error during root cause analysis: " + e.getMessage(), e);
88+
"Unexpected error during root cause analysis: " + McpResponseTrim.safeMessage(e), e);
8989
}
9090
}
9191

@@ -172,7 +172,8 @@ private Map<String, Object> buildDownstreamAnalysis(RcaRequest request) {
172172
addDownstreamEdges(downstreamAnalysis, downstreamResult, request.includeColumns());
173173
} catch (Exception e) {
174174
LOG.warn("Failed to perform downstream impact analysis for entity: {}", request.fqn(), e);
175-
downstreamAnalysis.put("error", "Failed to analyze downstream impact: " + e.getMessage());
175+
downstreamAnalysis.put(
176+
"error", "Failed to analyze downstream impact: " + McpResponseTrim.safeMessage(e));
176177
}
177178
return downstreamAnalysis;
178179
}
@@ -315,34 +316,30 @@ private static String relationshipType(Object pipeline) {
315316

316317
private static void applyDescription(Map<String, Object> slim, Object description) {
317318
if (description instanceof String text && !text.isEmpty()) {
318-
slim.put("description", truncate(text, TEXT_MAX_LENGTH));
319+
slim.put("description", McpResponseTrim.truncate(text, McpResponseTrim.TEXT_MAX_LENGTH));
319320
}
320321
}
321322

322323
private static void applySqlQuery(Map<String, Object> slim, Object sqlQuery) {
323324
if (sqlQuery instanceof String sql && !sql.isEmpty()) {
324-
slim.put("sqlQuery", truncate(sql, SQL_MAX_LENGTH));
325-
if (sql.length() > SQL_MAX_LENGTH) {
325+
slim.put("sqlQuery", McpResponseTrim.truncate(sql, McpResponseTrim.SQL_MAX_LENGTH));
326+
if (sql.length() > McpResponseTrim.SQL_MAX_LENGTH) {
326327
slim.put("sqlTruncated", Boolean.TRUE);
327328
}
328329
}
329330
}
330331

331332
private static void truncateDescriptionInPlace(Map<String, Object> map) {
332333
Object description = map.get("description");
333-
if (description instanceof String text && text.length() > TEXT_MAX_LENGTH) {
334-
map.put("description", truncate(text, TEXT_MAX_LENGTH));
334+
if (description instanceof String text && text.length() > McpResponseTrim.TEXT_MAX_LENGTH) {
335+
map.put("description", McpResponseTrim.truncate(text, McpResponseTrim.TEXT_MAX_LENGTH));
335336
}
336337
}
337338

338-
private static String truncate(String value, int maxLength) {
339-
return value.length() > maxLength ? value.substring(0, maxLength) + "..." : value;
340-
}
341-
342339
@VisibleForTesting
343340
static Map<String, Object> enforceSizeBudget(Map<String, Object> result) {
344341
Map<String, Object> output = result;
345-
if (JsonUtils.pojoToJson(result).length() > MAX_RESPONSE_CHARS) {
342+
if (McpResponseTrim.serializedLength(result) > McpResponseTrim.MAX_RESPONSE_CHARS) {
346343
output = oversizedHint(result);
347344
}
348345
return output;
@@ -417,36 +414,6 @@ private static void putIfPresent(Map<String, Object> map, String key, Object val
417414
}
418415
}
419416

420-
private static int parseIntParam(Object value, int defaultValue) {
421-
if (value == null) {
422-
return defaultValue;
423-
}
424-
if (value instanceof Number number) {
425-
return number.intValue();
426-
}
427-
if (value instanceof String string) {
428-
try {
429-
return Integer.parseInt(string);
430-
} catch (NumberFormatException e) {
431-
return defaultValue;
432-
}
433-
}
434-
return defaultValue;
435-
}
436-
437-
private static boolean parseBooleanParam(Object value, boolean defaultValue) {
438-
if (value == null) {
439-
return defaultValue;
440-
}
441-
if (value instanceof Boolean bool) {
442-
return bool;
443-
}
444-
if (value instanceof String string) {
445-
return "true".equalsIgnoreCase(string);
446-
}
447-
return defaultValue;
448-
}
449-
450417
@Override
451418
public Map<String, Object> execute(
452419
Authorizer authorizer,

0 commit comments

Comments
 (0)