Skip to content

Commit 37df493

Browse files
jamieQcodex
andauthored
fix(search-events): Allow final response after validation at step limit (#1349)
The "embedded model" for `search_events` has a limit to how many times it can try to come up with a valid search query (currently 5 model generations). If the model produces a query candidate that passes local validation but it happens to be on its final attempt, that potentially valid output is just dropped and we fail with a "unable to generate a query" sort of error (see a prod example trace [here](https://sentry.sentry.io/explore/logs/trace/c96c2679534a4858963680e943242586/?tab=ai-spans)). This change allows a query candidate which passes validation on the final step to use one more model invocation with tools removed to produce a "final answer" that we will then submit to the Sentry API (assuming it still passes validation). There might be other, better ways to do this, but it seemed like a fairly simple thing to try. Co-authored-by: Codex <noreply@openai.com>
1 parent 0563bde commit 37df493

5 files changed

Lines changed: 216 additions & 6 deletions

File tree

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { tool } from "ai";
2+
import { MockLanguageModelV3 } from "ai/test";
3+
import { expect, it, vi } from "vitest";
4+
import { z } from "zod";
5+
import { hasSuccessfulSearchValidation } from "../../tools/support/search-events/agent";
6+
import { callEmbeddedAgent } from "./callEmbeddedAgent";
7+
import { getAgentProvider } from "./provider-factory";
8+
9+
vi.mock("./provider-factory", () => ({ getAgentProvider: vi.fn() }));
10+
vi.mock("../../telem/logging", () => ({
11+
logIssue: vi.fn(),
12+
logWarn: vi.fn(),
13+
}));
14+
15+
const usage = {
16+
inputTokens: {
17+
total: 1,
18+
noCache: 1,
19+
cacheRead: 0,
20+
cacheWrite: 0,
21+
},
22+
outputTokens: { total: 1, text: 1, reasoning: 0 },
23+
};
24+
25+
function setupAgent(
26+
validationPassesOnLastStep: boolean,
27+
finalStepValidationCount = 1,
28+
) {
29+
let callCount = 0;
30+
const model = new MockLanguageModelV3({
31+
doGenerate: async () => {
32+
const step = ++callCount;
33+
if (step <= 5) {
34+
return {
35+
content: Array.from(
36+
{ length: step === 5 ? finalStepValidationCount : 1 },
37+
(_, candidate) => ({
38+
type: "tool-call" as const,
39+
toolCallId: `validation-${step}-${candidate}`,
40+
toolName: "validateSearch",
41+
input: JSON.stringify({ step, candidate }),
42+
}),
43+
),
44+
finishReason: { unified: "tool-calls" as const, raw: undefined },
45+
usage,
46+
warnings: [],
47+
};
48+
}
49+
return {
50+
content: [{ type: "text" as const, text: '{"result":"ready"}' }],
51+
finishReason: { unified: "stop" as const, raw: undefined },
52+
usage,
53+
warnings: [],
54+
};
55+
},
56+
});
57+
vi.mocked(getAgentProvider).mockReturnValue({
58+
type: "openai",
59+
label: "mock",
60+
getModel: () => model,
61+
getProviderOptions: () => ({}),
62+
});
63+
64+
const validateSearch = tool({
65+
inputSchema: z.object({ step: z.number(), candidate: z.number() }),
66+
execute: async ({ step }) => ({
67+
result: { valid: validationPassesOnLastStep && step === 5 },
68+
}),
69+
});
70+
71+
return { model, validateSearch };
72+
}
73+
74+
it("returns a structured answer after three validations pass on the fifth step", async () => {
75+
const { model, validateSearch } = setupAgent(true, 3);
76+
77+
const result = await callEmbeddedAgent({
78+
system: "Translate the query",
79+
prompt: "Test query",
80+
tools: { validateSearch },
81+
schema: z.object({ result: z.string() }),
82+
isReadyToFinalize: ({ toolResults }) =>
83+
hasSuccessfulSearchValidation(toolResults),
84+
});
85+
86+
expect(result.result).toEqual({ result: "ready" });
87+
expect(result.toolCalls).toHaveLength(7);
88+
expect(model.doGenerateCalls).toHaveLength(6);
89+
expect(model.doGenerateCalls[5]?.toolChoice).toEqual({ type: "none" });
90+
});
91+
92+
it("stops after five steps when the final validation fails", async () => {
93+
const { model, validateSearch } = setupAgent(false);
94+
95+
await expect(
96+
callEmbeddedAgent({
97+
system: "Translate the query",
98+
prompt: "Test query",
99+
tools: { validateSearch },
100+
schema: z.object({ result: z.string() }),
101+
isReadyToFinalize: ({ toolResults }) =>
102+
hasSuccessfulSearchValidation(toolResults),
103+
}),
104+
).rejects.toThrow("could not construct a valid query");
105+
106+
expect(model.doGenerateCalls).toHaveLength(5);
107+
});

‎packages/mcp-core/src/internal/agents/callEmbeddedAgent.test.ts‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,23 @@
1-
import { describe, it, expect, vi, beforeEach } from "vitest";
21
import {
3-
generateText,
42
APICallError,
3+
generateText,
4+
type LanguageModelUsage,
55
NoObjectGeneratedError,
66
NoOutputGeneratedError,
77
RetryError,
8-
type LanguageModelUsage,
8+
type StepResult,
9+
type Tool,
910
} from "ai";
11+
import { beforeEach, describe, expect, it, vi } from "vitest";
1012
import { z } from "zod";
11-
import { callEmbeddedAgent } from "./callEmbeddedAgent";
1213
import {
1314
AgentExecutionError,
1415
ConfigurationError,
1516
LLMProviderError,
1617
UserInputError,
1718
} from "../../errors";
1819
import { logIssue, logWarn } from "../../telem/logging";
20+
import { callEmbeddedAgent } from "./callEmbeddedAgent";
1921
import { getAgentProvider } from "./provider-factory";
2022

2123
vi.mock("../../telem/logging", () => ({
@@ -66,6 +68,31 @@ describe("callEmbeddedAgent", () => {
6668
process.env.OPENROUTER_API_KEY = "";
6769
});
6870

71+
it("keeps the five-step limit when no finalization condition is supplied", async () => {
72+
mockGenerateText.mockResolvedValue({
73+
experimental_output: { result: "ok" },
74+
} as never);
75+
76+
await callEmbeddedAgent({
77+
system: "You are a test agent",
78+
prompt: "Test prompt",
79+
tools: {},
80+
schema: testSchema,
81+
});
82+
83+
const stopWhen = mockGenerateText.mock.calls[0]?.[0]?.stopWhen;
84+
if (typeof stopWhen !== "function") {
85+
throw new Error("Expected a stop condition");
86+
}
87+
88+
const step = { toolResults: [{ output: false }] } as StepResult<
89+
Record<string, Tool>
90+
>;
91+
const firstFour = Array.from({ length: 4 }, () => step);
92+
expect(stopWhen({ steps: firstFour })).toBe(false);
93+
expect(stopWhen({ steps: [...firstFour, step] })).toBe(true);
94+
});
95+
6996
it("throws LLMProviderError for OpenAI region restriction", async () => {
7097
// Create an APICallError simulating OpenAI's region restriction
7198
const regionError = new APICallError({

‎packages/mcp-core/src/internal/agents/callEmbeddedAgent.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
NoOutputGeneratedError,
66
Output,
77
RetryError,
8-
stepCountIs,
8+
type StepResult,
99
type Tool,
1010
} from "ai";
1111
import type { z } from "zod";
@@ -18,6 +18,9 @@ import {
1818
import { logIssue, logWarn } from "../../telem/logging";
1919
import { getAgentProvider } from "./provider-factory";
2020

21+
// Tool work gets this many model steps; a validated final response may get one more.
22+
const MAX_WORK_STEPS = 5;
23+
2124
/**
2225
* Resolve the underlying provider failure from an AI SDK error.
2326
*
@@ -112,14 +115,25 @@ export async function callEmbeddedAgent<
112115
prompt,
113116
tools,
114117
schema,
118+
isReadyToFinalize,
115119
}: {
116120
system: string;
117121
prompt: string;
118122
tools: Record<string, Tool>;
119123
schema: TSchema;
124+
/** Whether this step produced a result that permits a final response. */
125+
isReadyToFinalize?: (step: StepResult<Record<string, Tool>>) => boolean;
120126
}): Promise<EmbeddedAgentResult<TOutput>> {
121127
const capturedToolCalls: ToolCall[] = [];
122128

129+
function shouldAllowFinalResponseStep(
130+
steps: StepResult<Record<string, Tool>>[],
131+
): boolean {
132+
if (steps.length !== MAX_WORK_STEPS) return false;
133+
const lastStep = steps.at(-1);
134+
return lastStep !== undefined && (isReadyToFinalize?.(lastStep) ?? false);
135+
}
136+
123137
// Get the configured provider (OpenAI, Azure OpenAI, or Anthropic)
124138
const provider = getAgentProvider();
125139

@@ -129,7 +143,12 @@ export async function callEmbeddedAgent<
129143
system,
130144
prompt,
131145
tools,
132-
stopWhen: stepCountIs(5),
146+
stopWhen: ({ steps }) =>
147+
steps.length >= MAX_WORK_STEPS && !shouldAllowFinalResponseStep(steps),
148+
prepareStep: ({ steps }) => {
149+
if (shouldAllowFinalResponseStep(steps)) return { toolChoice: "none" };
150+
return undefined;
151+
},
133152
experimental_output: Output.object({ schema }),
134153
experimental_telemetry: {
135154
isEnabled: true,
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { expect, it } from "vitest";
2+
import { hasSuccessfulSearchValidation } from "./agent";
3+
4+
it("identifies a passing validateSearch result", () => {
5+
expect(hasSuccessfulSearchValidation([])).toBe(false);
6+
expect(
7+
hasSuccessfulSearchValidation([
8+
{ toolName: "datasetAttributes", output: { result: {} } },
9+
]),
10+
).toBe(false);
11+
expect(
12+
hasSuccessfulSearchValidation([
13+
{ toolName: "validateSearch", output: { result: { valid: false } } },
14+
]),
15+
).toBe(false);
16+
expect(
17+
hasSuccessfulSearchValidation([
18+
{ toolName: "validateSearch", output: { error: "API unavailable" } },
19+
]),
20+
).toBe(false);
21+
expect(
22+
hasSuccessfulSearchValidation([
23+
{ toolName: "validateSearch", output: { result: { valid: true } } },
24+
]),
25+
).toBe(true);
26+
});
27+
28+
it("accepts any successful validation among candidates checked in one step", () => {
29+
const passing = {
30+
toolName: "validateSearch",
31+
output: { result: { valid: true } },
32+
};
33+
const failing = {
34+
toolName: "validateSearch",
35+
output: { result: { valid: false } },
36+
};
37+
38+
expect(hasSuccessfulSearchValidation([passing, failing])).toBe(true);
39+
expect(hasSuccessfulSearchValidation([failing, passing])).toBe(true);
40+
expect(hasSuccessfulSearchValidation([failing, failing])).toBe(false);
41+
});

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,20 @@ import {
1515
} from "./utils";
1616

1717
const SEARCH_EVENTS_DATASETS = [...PUBLIC_EVENTS_DATASETS, "replays"] as const;
18+
const successfulValidateSearchOutputSchema = z.object({
19+
result: z.object({ valid: z.literal(true) }),
20+
});
21+
22+
export function hasSuccessfulSearchValidation(
23+
toolResults: readonly { toolName: string; output: unknown }[],
24+
): boolean {
25+
// A step may validate several candidates; any successful one qualifies.
26+
return toolResults.some(
27+
({ toolName, output }) =>
28+
toolName === "validateSearch" &&
29+
successfulValidateSearchOutputSchema.safeParse(output).success,
30+
);
31+
}
1832

1933
// .default("") on explanation is safe because structuredOutputs: false is set via providerOptions.
2034
// If structuredOutputs is re-enabled, remove .default() calls (OpenAI requires all fields in 'required').
@@ -228,5 +242,7 @@ export async function searchEventsAgent(
228242
whoami: whoamiTool,
229243
},
230244
schema: searchEventsAgentOutputSchema,
245+
isReadyToFinalize: ({ toolResults }) =>
246+
hasSuccessfulSearchValidation(toolResults),
231247
});
232248
}

0 commit comments

Comments
 (0)