Skip to content

Commit c13c685

Browse files
jamieQcodexbetegon
authored
fix(search_events): Remove environment from agent validation tool (#1346)
Translating a user query into a valid search event API call seems to often fail. One reason that seems fairly common is that models end up hallucinating invalid `environment` parameters. E.g. see [this representative trace](https://sentry.sentry.io/explore/logs/trace/c96c2679534a4858963680e943242586/?tab=ai-spans) in which a model repeatedly tries to use `":/"` as an environment value. Sometimes the models correct these mistakes before the retry budget is spent, but other times this just results in a terminal failure. Try to fix this by removing the `environment` parameter from the `createValidateEventsSearchTool` "schema" so that models are (hopefully) less likely to fill that field with garbage values. AFAICT the field is unnecessary for internal validation, since the env should be passed via the `query` parameter for most datasets. Replays is apparently an exception, but I don't believe it depends on this validation logic currently so should be unaffected. Local testing suggested this improved the model behavior in some cases that were modeled after production failures. --------- Co-authored-by: GPT-6 <noreply@openai.com> Co-authored-by: betegon <miguelbetegongarcia@gmail.com>
1 parent e6cde68 commit c13c685

7 files changed

Lines changed: 213 additions & 42 deletions

File tree

‎docs/specs/search-events.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,21 @@ The AI produces different query patterns based on the selected dataset:
9595
- **Logs dataset**: Focus on `message`, `severity`, `severity_number`, **NO timestamp filters** (uses statsPeriod instead)
9696
- **Tracemetrics dataset**: Focus on `metric.name`, `metric.type`, `metric.unit`, `value`, and metric-aware aggregates like `p95(value,http.request.duration,distribution,millisecond)`
9797

98+
### Environment Filters
99+
100+
The embedded agent receives known visible environment names as context. It must
101+
only add an environment filter when requested; a single available environment or
102+
grouping by environment does not imply a filter. These instructions also apply
103+
when discovery fails or the list is too large to include in the prompt.
104+
105+
For non-replay datasets, environment filters belong in `query`; the agent leaves
106+
its separate `environment` output null. The internal `validateSearch` tool accepts
107+
the candidate query without a separate environment argument. Replays retain the
108+
separate environment parameter and do not use this validation tool.
109+
110+
The discovered list is not an exhaustive allowlist: hidden environments can be
111+
absent. Existing final validation and unknown-environment notices remain in place.
112+
98113
### Time Series
99114

100115
Requests for a metric over time ("per hour", "per day", "trend", "over time") return a bucketed series via the `events-stats` endpoint instead of failing.

‎packages/mcp-core/src/tools/catalog/search-events.test.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3133,6 +3133,7 @@ describe("search_events", () => {
31333133

31343134
it("merges agent environment into the events search query for non-replay datasets", async () => {
31353135
let eventsRequestUrl: URL | undefined;
3136+
let validationRequestUrl: URL | undefined;
31363137

31373138
mockGenerateText.mockResolvedValueOnce(
31383139
mockAIResponse(
@@ -3149,7 +3150,10 @@ describe("search_events", () => {
31493150
mswServer.use(
31503151
http.get(
31513152
"https://sentry.io/api/0/organizations/test-org/events/validate/",
3152-
() => HttpResponse.json(validEventsValidationResponse),
3153+
({ request }) => {
3154+
validationRequestUrl = new URL(request.url);
3155+
return HttpResponse.json(validEventsValidationResponse);
3156+
},
31533157
),
31543158
http.get(
31553159
"https://sentry.io/api/0/organizations/test-org/events/",
@@ -3190,6 +3194,9 @@ describe("search_events", () => {
31903194
expect(eventsRequestUrl!.searchParams.get("query")).toContain(
31913195
"environment:production",
31923196
);
3197+
expect(validationRequestUrl!.searchParams.get("environment")).toBe(
3198+
"production",
3199+
);
31933200
});
31943201

31953202
it("does not call events when final validation fails", async () => {

‎packages/mcp-core/src/tools/support/search-events/agent.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const searchEventsAgentOutputSchema = z
3434
.nullable()
3535
.default(null)
3636
.describe(
37-
"Separate environment filter for datasets like replays that do not support environment in the query string. Set only to a real environment the user named (see the 'Available environments' list); omit otherwise. Never use wildcards, placeholders, or example values.",
37+
"Environment filter for replays only, when requested. Otherwise null; non-replay filters belong in query.",
3838
),
3939
timeSeries: z
4040
.object({
@@ -120,9 +120,8 @@ export interface SearchEventsAgentOptions {
120120
environmentNames?: string[];
121121
}
122122

123-
// Above this many environments we stop inlining the full list into the prompt
124-
// (token cost) and rely on the guidance text alone; validation still checks the
125-
// value against the real list.
123+
// Above this many environments we omit the names to bound prompt size. The
124+
// environment filtering instructions still apply without the list.
126125
const MAX_INLINE_ENVIRONMENTS = 100;
127126

128127
/**
@@ -137,16 +136,18 @@ export function buildSystemPromptWithEnvironments(
137136
base: string,
138137
environmentNames: string[],
139138
): string {
139+
const rule =
140+
"Filter by environment only when requested. Use query filters for non-replays; reserve `environment` for replays and set it to null otherwise. Never invent values; grouping or availability alone does not request a filter.";
140141
if (environmentNames.length === 0) {
141-
return base;
142+
return `${base}\n\n## Environment filters\n${rule}`;
142143
}
143-
const rule =
144-
'When the user names an environment, set the `environment` field to a matching value from this list EXACTLY; otherwise OMIT the field. Never use wildcards, placeholders, "null", "*", empty strings, or example values.';
145144
if (environmentNames.length <= MAX_INLINE_ENVIRONMENTS) {
146-
const list = environmentNames.map((name) => `"${name}"`).join(", ");
147-
return `${base}\n\n## Available environments\nThe only valid \`environment\` values for this organization are: ${list}.\n${rule}`;
145+
const list = environmentNames
146+
.map((name) => JSON.stringify(name))
147+
.join(", ");
148+
return `${base}\n\n## Available environments\nVisible environments: ${list}. Hidden environments may be absent.\n${rule}`;
148149
}
149-
return `${base}\n\n## Environments\nThis organization has ${environmentNames.length} environments. ${rule}`;
150+
return `${base}\n\n## Environments\n${environmentNames.length} visible environments (list omitted). ${rule}`;
150151
}
151152

152153
/**

‎packages/mcp-core/src/tools/support/search-events/environments.test.ts‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,19 @@ import { SentryApiService } from "../../../api-client";
33
import { buildSystemPromptWithEnvironments } from "./agent";
44

55
describe("buildSystemPromptWithEnvironments", () => {
6-
it("returns the base prompt unchanged when there are no environments", () => {
7-
expect(buildSystemPromptWithEnvironments("BASE", [])).toBe("BASE");
8-
});
9-
10-
it("inlines a small list of real environment names with a guardrail", () => {
6+
it("inlines a small list of real environment names", () => {
117
const out = buildSystemPromptWithEnvironments("BASE", [
128
"production",
139
"dev",
1410
]);
1511
expect(out).toContain("BASE");
16-
expect(out).toContain("Available environments");
1712
expect(out).toContain('"production"');
1813
expect(out).toContain('"dev"');
19-
// Guardrail against the hallucinated placeholders that caused the failures.
20-
expect(out).toContain("OMIT the field");
21-
expect(out).toContain("Never use wildcards");
2214
});
2315

2416
it("does not dump the full list for very large orgs", () => {
2517
const many = Array.from({ length: 250 }, (_, i) => `env-${i}`);
2618
const out = buildSystemPromptWithEnvironments("BASE", many);
27-
expect(out).toContain("250 environments");
2819
expect(out).not.toContain('"env-0"');
2920
});
3021
});

‎packages/mcp-core/src/tools/support/search-events/utils.test.ts‎

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { mswServer } from "@sentry/mcp-server-mocks";
22
import { HttpResponse, http } from "msw";
33
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
4+
import type { z } from "zod";
45
import { SentryApiService } from "../../../api-client";
56
import * as logging from "../../../telem/logging";
67
import {
8+
createValidateEventsSearchTool,
79
fetchCustomAttributes,
810
formatEventsValidationResults,
911
formatEventValue,
@@ -12,6 +14,78 @@ import {
1214
looksLikeSentrySearchSyntax,
1315
} from "./utils";
1416

17+
describe("validateSearch tool contract", () => {
18+
it("does not expose a separate environment argument", () => {
19+
const tool = createValidateEventsSearchTool({
20+
apiService: new SentryApiService({ accessToken: "test-token" }),
21+
organizationSlug: "test-org",
22+
});
23+
24+
const schema = tool.inputSchema as z.ZodObject;
25+
expect(Object.keys(schema.shape)).not.toContain("environment");
26+
});
27+
28+
it.each([
29+
{
30+
name: "does not add an environment filter when none is requested",
31+
query: "span.duration:>100",
32+
},
33+
{
34+
name: "preserves environment filters in the candidate query",
35+
query: "span.duration:>100 environment:production",
36+
},
37+
{
38+
name: "does not forward a hallucinated separate environment argument",
39+
query: "span.duration:>100",
40+
environment: ":/",
41+
},
42+
])("$name", async ({ query, environment }) => {
43+
const requests: URLSearchParams[] = [];
44+
mswServer.use(
45+
http.get(
46+
"https://sentry.io/api/0/organizations/test-org/events/validate/",
47+
({ request }) => {
48+
requests.push(new URL(request.url).searchParams);
49+
return HttpResponse.json({
50+
valid: true,
51+
projects: [],
52+
dataset: [],
53+
environment: [],
54+
field: [],
55+
query: { valid: true, error: null, fields: [] },
56+
orderby: [],
57+
});
58+
},
59+
),
60+
);
61+
const tool = createValidateEventsSearchTool({
62+
apiService: new SentryApiService({ accessToken: "test-token" }),
63+
organizationSlug: "test-org",
64+
});
65+
type ToolInput = Parameters<NonNullable<typeof tool.execute>>[0];
66+
const schema = tool.inputSchema as z.ZodType<ToolInput>;
67+
const input = schema.parse({
68+
dataset: "spans",
69+
query,
70+
fields: ["span.duration"],
71+
sort: "-span.duration",
72+
environment,
73+
});
74+
75+
const result = await tool.execute!(input, {
76+
toolCallId: "validate-search-test",
77+
messages: [],
78+
});
79+
80+
expect(requests).toHaveLength(1);
81+
expect(requests[0]!.getAll("environment")).toEqual([]);
82+
expect(requests[0]!.get("query")).toBe(query);
83+
expect(result).toEqual({
84+
result: { valid: true, message: "Search validation passed." },
85+
});
86+
});
87+
});
88+
1589
describe("formatEventValue", () => {
1690
describe("primitives", () => {
1791
it("should return 'null' for null", () => {
@@ -313,12 +387,10 @@ describe("search query helpers", () => {
313387

314388
// Bare substring matches are not enough — short values must not false-hit
315389
// inside unrelated full-text (e.g. "1" inside "401").
316-
expect(
317-
isSemanticFilterDowngrade("id:1", 'message:"error 401"'),
318-
).toBe(false);
319-
expect(
320-
isSemanticFilterDowngrade("id:1", 'message:"error 1"'),
321-
).toBe(true);
390+
expect(isSemanticFilterDowngrade("id:1", 'message:"error 401"')).toBe(
391+
false,
392+
);
393+
expect(isSemanticFilterDowngrade("id:1", 'message:"error 1"')).toBe(true);
322394
});
323395
});
324396

‎packages/mcp-core/src/tools/support/search-events/utils.ts‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,10 @@ function normalizeFilterValue(rawValue: string): string {
177177
* Read one filter value starting at `valueStart` in the original query.
178178
* Quoted values keep interior whitespace; unquoted values stop at whitespace.
179179
*/
180-
function readRawFilterValue(query: string, valueStart: number): string | undefined {
180+
function readRawFilterValue(
181+
query: string,
182+
valueStart: number,
183+
): string | undefined {
181184
if (valueStart >= query.length) {
182185
return undefined;
183186
}
@@ -241,9 +244,7 @@ function searchFilterOccurrences(query: string): SearchFilterOccurrence[] {
241244
return occurrences;
242245
}
243246

244-
function structuredFilterOccurrences(
245-
query: string,
246-
): SearchFilterOccurrence[] {
247+
function structuredFilterOccurrences(query: string): SearchFilterOccurrence[] {
247248
return searchFilterOccurrences(query).filter(
248249
(occurrence) => !FULL_TEXT_SEARCH_KEYS.has(occurrence.key),
249250
);
@@ -284,9 +285,7 @@ function containsAsWholeToken(haystack: string, needle: string): boolean {
284285
}
285286

286287
function isRelatedFilterValue(left: string, right: string): boolean {
287-
return (
288-
containsAsWholeToken(left, right) || containsAsWholeToken(right, left)
289-
);
288+
return containsAsWholeToken(left, right) || containsAsWholeToken(right, left);
290289
}
291290

292291
/**
@@ -1097,12 +1096,6 @@ export function createValidateEventsSearchTool(options: {
10971096
.describe("Optional relative time period like 1h, 24h, 7d"),
10981097
start: z.string().optional().describe("Optional ISO 8601 start time"),
10991098
end: z.string().optional().describe("Optional ISO 8601 end time"),
1100-
environment: z
1101-
.union([z.string().min(1), z.array(z.string().min(1)).min(1)])
1102-
.optional()
1103-
.describe(
1104-
"Optional environment filter. Prefer query filters for non-replay datasets.",
1105-
),
11061099
}),
11071100
execute: async ({
11081101
dataset,
@@ -1112,7 +1105,6 @@ export function createValidateEventsSearchTool(options: {
11121105
statsPeriod,
11131106
start,
11141107
end,
1115-
environment,
11161108
}) => {
11171109
const validation = await validateEventsSearch(apiService, {
11181110
organizationSlug,
@@ -1121,7 +1113,6 @@ export function createValidateEventsSearchTool(options: {
11211113
query,
11221114
sort,
11231115
projectId,
1124-
environment,
11251116
statsPeriod,
11261117
start,
11271118
end,

‎packages/mcp-server-evals/src/evals/search-events-environments.eval.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { SentryApiService } from "@sentry/mcp-core/api-client";
2+
import { searchEventsAgent } from "@sentry/mcp-core/tools/search-events/agent";
3+
import { expect } from "vitest";
4+
import { describeEval } from "vitest-evals";
5+
import "../setup-env";
6+
import { StructuredOutputScorer } from "./utils/structuredOutputScorer";
7+
8+
describeEval("search-events-environment-grounding", {
9+
data: async () =>
10+
["message:*decoder*", "message:*decoder* environment:production"].map(
11+
(query) => ({
12+
// Sanitized request shape from a real failure: a structured errors
13+
// aggregate exhausted all five steps on invented environment values,
14+
// despite the prompt already listing the only real environment.
15+
// Keep the handler's request wrapper, including environment: null.
16+
input: [
17+
"Translate this Sentry event search request.",
18+
"The query may be natural language or already-valid Sentry search syntax.",
19+
"Preserve valid explicit parameters, but correct dataset, query syntax, fields, sort, and time range when they conflict or would fail.",
20+
"If the user query already uses Sentry search syntax, treat its filters as authoritative unless validateSearch proves a field is invalid.",
21+
"Never replace a structured field filter with message/log.body/full-text matching. If no valid attribute exists for an explicit field:value filter, keep the field and let validation fail.",
22+
"For spans, logs, and metrics, use datasetAttributes to discover likely fields with substringMatch, query, and attributeTypes before dropping or renaming explicit fields.",
23+
"A broad datasetAttributes result may be truncated, so absence from that preview does not prove an explicit field is invalid.",
24+
"For non-replay datasets, call validateSearch on the candidate request and fix failures in this same pass before returning.",
25+
"For non-replay datasets, convert environment parameters into query filters. For replays, keep environment in the separate environment parameter.",
26+
"",
27+
`User query: ${query}`,
28+
"Current parameters:",
29+
JSON.stringify(
30+
{
31+
dataset: "errors",
32+
fields: ["count()"],
33+
sort: "-count()",
34+
statsPeriod: "90d",
35+
environment: null,
36+
},
37+
null,
38+
2,
39+
),
40+
].join("\n"),
41+
expected: {
42+
dataset: "errors",
43+
fields: ["count()"],
44+
sort: "-count()",
45+
environment: null,
46+
timeRange: { statsPeriod: "90d" },
47+
},
48+
}),
49+
),
50+
task: async (input) => {
51+
const agentResult = await searchEventsAgent({
52+
query: input,
53+
organizationSlug: "sentry-mcp-evals",
54+
apiService: new SentryApiService({ accessToken: "test-token" }),
55+
environmentNames: ["production"],
56+
});
57+
58+
const validationCalls = agentResult.toolCalls.filter(
59+
(call) => call.toolName === "validateSearch",
60+
);
61+
expect(validationCalls.length).toBeGreaterThan(0);
62+
63+
const queries = [agentResult.result.query];
64+
for (const call of validationCalls) {
65+
expect(call.args).toBeTypeOf("object");
66+
expect(call.args).not.toBeNull();
67+
expect(call.args).not.toHaveProperty("environment");
68+
const args = call.args as Record<string, unknown>;
69+
expect(args.query).toBeTypeOf("string");
70+
queries.push(args.query as string);
71+
}
72+
73+
// Check every attempted validation, not just the final successful query:
74+
// repeated invalid tool calls can consume the budget before any output.
75+
const requestedEnvironment = input.includes("environment:production");
76+
for (const query of queries) {
77+
expect(query).toContain("message:*decoder*");
78+
if (requestedEnvironment) {
79+
expect(query).toMatch(
80+
/(?:^|\s)environment:(?:production|"production")(?=\s|$)/,
81+
);
82+
expect(query.match(/\benvironment\s*:/g)).toHaveLength(1);
83+
} else {
84+
expect(query).not.toMatch(/\benvironment\s*:/);
85+
}
86+
}
87+
expect(agentResult.result.environment).toBeNull();
88+
89+
return { result: JSON.stringify(agentResult.result) };
90+
},
91+
scorers: [StructuredOutputScorer()],
92+
threshold: 1,
93+
timeout: 60000,
94+
});

0 commit comments

Comments
 (0)