fix(ai): warn first when an MCP call's argument was ignored

apps/ai/scripts/pr-review-local.ts

@@ -58,7 +58,7 @@ import { AGENTS } from "@/chat/agents"
 import type { ChatTurnEvent } from "@/chat/events"
 import { withToolTranscript } from "@/chat/close-out"
 import { PR_REVIEW_TOOLS } from "@/chat/permissions"
-import { normalizeArguments, argumentNotices } from "@/mcp/lib/decode-issues"
+import { normalizeArguments, argumentWarnings } from "@/mcp/lib/decode-issues"
 import { PR_REVIEW_CLOSE_OUT_PROMPT } from "@/chat/prompts"
 import { makeReviewCoverage } from "@/chat/review-coverage"
 import { makeReviewLedger } from "@/chat/review-ledger"
@@ -766,9 +766,9 @@ export const makeExecutor = (input: {
 				return {
 					...result,
 					content: [
-						...argumentNotices(normalized, name).map((notice) => ({
+						...argumentWarnings(normalized, name).map((warning) => ({
 							type: "text" as const,
-							text: notice,
+							text: warning,
 						})),
 						...result.content,
 					],

apps/ai/src/mcp/__evals__/tool-contract.test.ts

@@ -38,6 +38,8 @@ beforeAll(() => {
 			rows: [],
 		},
 		...rules,
+		// Any other query (list_services) returns no rows.
+		{ match: () => true, rows: [] },
 	])
 	rt = makeEvalRuntime()
 })
@@ -81,7 +83,15 @@ describe("a tool declared with define", () => {
 	it("tells the model which keys it ignored, and what it probably meant", async () => {
 		const result = await call("find_errors", { servce: "checkout" })
 		expect(markdown(result)).toContain(
-			"Note: `servce` is not a parameter of `find_errors` and was ignored. Did you mean `service`?",
+			"Warning: `servce` is not a parameter of `find_errors`. It was ignored, and this result is for the call without it. Did you mean `service`?",
+		)
+	})
+
+	it("warns first, under the title, when a window key it does not take was ignored", async () => {
+		const result = await call("list_services", { time_range: "today" })
+		expect(result.isError).toBeUndefined()
+		expect(markdown(result)).toMatch(
+			/^## Services\n\nWarning: `time_range` is not a parameter of `list_services`\. It was ignored, and this result is for the call without it\. Did you mean `start_time`\/`end_time`\?\n\nTime range: /,
 		)
 	})
 

apps/ai/src/mcp/lib/decode-issues.test.ts

@@ -1,7 +1,7 @@
 import { describe, expect, it } from "vitest"
 import { Exit, Schema } from "effect"
 import { toInputSchema } from "../tools/registry"
-import { formatDecodeFailure, normalizeArguments, suggestParameter } from "./decode-issues"
+import { argumentWarnings, formatDecodeFailure, normalizeArguments, suggestParameter } from "./decode-issues"
 import * as P from "./params"
 
 const failureFor = (tool: string, schema: Schema.Codec<unknown, unknown, never, unknown>, input: unknown) => {
@@ -81,7 +81,34 @@ describe("argument normalization", () => {
 	it("drops unknown keys and reports them", () => {
 		const normalized = normalizeArguments({ servce: "api", limit: 5 }, ["service", "limit"])
 		expect(normalized.args).toEqual({ limit: 5 })
-		expect(normalized.unknown).toEqual([{ key: "servce", suggestion: "service" }])
+		expect(normalized.unknown).toEqual([{ key: "servce", suggestions: ["service"] }])
+	})
+
+	it("points a key that names a whole window at both bounds", () => {
+		const known = ["service", "start_time", "end_time", "limit"]
+		const unknown = (key: string) => normalizeArguments({ [key]: "today" }, known).unknown
+		expect(unknown("time_range")).toEqual([
+			{ key: "time_range", suggestions: ["start_time", "end_time"] },
+		])
+		expect(unknown("lookback")).toEqual([{ key: "lookback", suggestions: ["start_time", "end_time"] }])
+		// One bound stays one bound.
+		expect(unknown("start")).toEqual([{ key: "start", suggestions: ["start_time"] }])
+		// A closer match that is not a bound wins over the window guess.
+		const withDuration = [...known, "min_duration_ms", "max_duration_ms"]
+		expect(normalizeArguments({ min_duration: 5 }, withDuration).unknown).toEqual([
+			{ key: "min_duration", suggestions: ["min_duration_ms"] },
+		])
+		// A tool without a window gets no window suggestion.
+		expect(normalizeArguments({ time_range: "1h" }, ["service"]).unknown).toEqual([
+			{ key: "time_range", suggestions: [] },
+		])
+	})
+
+	it("warns that the result is for the call without the ignored key", () => {
+		const normalized = normalizeArguments({ time_range: "today" }, ["start_time", "end_time"])
+		expect(argumentWarnings(normalized, "list_services")).toEqual([
+			"`time_range` is not a parameter of `list_services`. It was ignored, and this result is for the call without it. Did you mean `start_time`/`end_time`?",
+		])
 	})
 
 	it("fixes the case of an enum value when exactly one published value matches", () => {

apps/ai/src/mcp/lib/decode-issues.ts

@@ -15,8 +15,14 @@ export interface NormalizedArguments {
 	readonly args: unknown
 	/** Retired names that were rewritten to their current name. */
 	readonly renamed: ReadonlyArray<readonly [from: string, to: string]>
-	/** Keys the schema does not have, with the parameter each was probably meant to be. */
-	readonly unknown: ReadonlyArray<{ readonly key: string; readonly suggestion?: string }>
+	/** Keys the schema does not have, with the parameters each was probably meant to be. */
+	readonly unknown: ReadonlyArray<UnknownArgument>
+}
+
+export interface UnknownArgument {
+	readonly key: string
+	/** Empty when no parameter is close; both bounds for a key that names a whole window. */
+	readonly suggestions: ReadonlyArray<string>
 }
 
 const isPlainObject = (value: unknown): value is Readonly<Record<string, unknown>> =>
@@ -72,6 +78,43 @@ export const suggestParameter = (key: string, known: ReadonlyArray<string>): str
 	return best?.name
 }
 
+/** Words that name a whole time window (`time_range`, `lookback`) rather than one of its bounds. */
+const WINDOW_WORDS: ReadonlySet<string> = new Set([
+	"range",
+	"window",
+	"timerange",
+	"timeframe",
+	"timespan",
+	"period",
+	"since",
+	"lookback",
+	"duration",
+	"last",
+	"ago",
+])
+
+const isWindowKey = (key: string): boolean =>
+	key.toLowerCase() === "time" || words(key).some((word) => WINDOW_WORDS.has(word))
+
+const WINDOW_BOUNDS: ReadonlyArray<string> = ["start_time", "end_time"]
+
+/**
+ * The parameters an unknown key most plausibly meant: `time_range` is both window bounds. A closer
+ * match that is not a bound wins, so `min_duration` still points at `min_duration_ms`.
+ */
+const suggestParameters = (key: string, known: ReadonlyArray<string>): ReadonlyArray<string> => {
+	const suggestion = suggestParameter(key, known)
+	const widens =
+		(suggestion === undefined || WINDOW_BOUNDS.includes(suggestion)) &&
+		isWindowKey(key) &&
+		WINDOW_BOUNDS.every((bound) => known.includes(bound))
+	if (widens) return WINDOW_BOUNDS
+	return suggestion === undefined ? [] : [suggestion]
+}
+
+const didYouMean = (suggestions: ReadonlyArray<string>): string =>
+	suggestions.length === 0 ? "" : ` Did you mean ${suggestions.map((name) => `\`${name}\``).join("/")}?`
+
 /**
  * Apply aliases and drop keys the schema does not declare. A dropped key is reported rather than
  * silently ignored: a model that sent `service_name` to a tool without it believed it filtered.
@@ -110,7 +153,7 @@ export const normalizeArguments = (
 	const knownSet = new Set(known)
 	const args: Record<string, unknown> = {}
 	const renamed: Array<readonly [string, string]> = []
-	const unknown: Array<{ key: string; suggestion?: string }> = []
+	const unknown: Array<UnknownArgument> = []
 	for (const [key, value] of Object.entries(input)) {
 		if (knownSet.has(key)) {
 			args[key] = canonicalEnumValue(value, enums.get(key))
@@ -123,20 +166,20 @@ export const normalizeArguments = (
 			renamed.push([key, target])
 			continue
 		}
-		const suggestion = suggestParameter(key, known)
-		unknown.push(suggestion === undefined ? { key } : { key, suggestion })
+		unknown.push({ key, suggestions: suggestParameters(key, known) })
 	}
 	return { args, renamed, unknown }
 }
 
-/** Notes for the result text: how the call was read, when that differs from what was sent. */
-export const argumentNotices = (normalized: NormalizedArguments, tool: string): ReadonlyArray<string> => [
-	...normalized.unknown.map(({ key, suggestion }) =>
-		suggestion === undefined
-			? `\`${key}\` is not a parameter of \`${tool}\` and was ignored.`
-			: `\`${key}\` is not a parameter of \`${tool}\` and was ignored. Did you mean \`${suggestion}\`?`,
-	),
-]
+/**
+ * Warnings for the result text, one per ignored key. The call still ran, so each says the result
+ * is for the call without that key: a model that sent `time_range` otherwise reads a default window.
+ */
+export const argumentWarnings = (normalized: NormalizedArguments, tool: string): ReadonlyArray<string> =>
+	normalized.unknown.map(
+		({ key, suggestions }) =>
+			`\`${key}\` is not a parameter of \`${tool}\`. It was ignored, and this result is for the call without it.${didYouMean(suggestions)}`,
+	)
 
 interface ParameterDoc {
 	readonly type: string
@@ -216,10 +259,8 @@ export const formatDecodeFailure = (
 			? `- Missing required ${describeParameter(name, docs)}`
 			: `- \`${name}\`${nested}: ${message}`
 	})
-	const ignored = normalized.unknown.map(({ key, suggestion }) =>
-		suggestion === undefined
-			? `- \`${key}\` is not a parameter of this tool.`
-			: `- \`${key}\` is not a parameter of this tool. Did you mean \`${suggestion}\`?`,
+	const ignored = normalized.unknown.map(
+		({ key, suggestions }) => `- \`${key}\` is not a parameter of this tool.${didYouMean(suggestions)}`,
 	)
 	const names = [...docs.keys()].map((name) => (required.has(name) ? `${name} (required)` : name))
 	return [

apps/ai/src/mcp/lib/tool-doc.test.ts

@@ -9,7 +9,8 @@ describe("renderToolDoc", () => {
 				["Time range", "a to b"],
 				["Service", undefined],
 			],
-			notices: ["`servce` is not a parameter"],
+			warnings: ["`servce` is not a parameter"],
+			notices: ["Window narrowed"],
 			blocks: [doc.table(["Error", "Count"], [["Boom | pipe", "3"]])],
 			truncation: {
 				shown: 1,
@@ -22,8 +23,9 @@ describe("renderToolDoc", () => {
 		expect(text).toBe(
 			[
 				"## Errors by Type",
+				"Warning: `servce` is not a parameter",
 				"Time range: a to b",
-				"Note: `servce` is not a parameter",
+				"Note: Window narrowed",
 				"| Error | Count |\n|---|---|\n| Boom \\| pipe | 3 |",
 				"Showing 1 of 9 error types. Next page: `find_errors offset=1`: the next page",
 				'Next:\n- `error_detail fingerprint="123"`: sample traces',

apps/ai/src/mcp/lib/tool-doc.ts

@@ -33,6 +33,8 @@ export type DocBlock =
 export interface ToolDoc {
 	/** Rendered as the `##` heading. */
 	readonly title: string
+	/** Where the call was not run as sent (ignored keys). First under the title, so it is not missed. */
+	readonly warnings?: ReadonlyArray<string>
 	/** What the result covers: the resolved window and the filters that applied. One line. */
 	readonly scope?: ReadonlyArray<readonly [string, string | undefined]>
 	readonly blocks: ReadonlyArray<DocBlock>
@@ -45,7 +47,7 @@ export interface ToolDoc {
 		readonly noun: string
 		readonly next?: NextCall
 	}
-	/** Things the model should know about how its call was read (ignored keys, clamped values). */
+	/** Things the model should know about how its call was read (clamped windows, inferred values). */
 	readonly notices?: ReadonlyArray<string>
 	readonly next?: ReadonlyArray<NextCall>
 }
@@ -122,6 +124,9 @@ const renderScope = (scope: NonNullable<ToolDoc["scope"]>): string | undefined =
 /** Markdown, in a fixed order, with blank lines between sections. */
 export const renderToolDoc = (tool: ToolDoc): string => {
 	const sections: Array<string> = [`## ${tool.title}`]
+	if (tool.warnings !== undefined && tool.warnings.length > 0) {
+		sections.push(tool.warnings.map((warning) => `Warning: ${warning}`).join("\n"))
+	}
 	const scope = tool.scope === undefined ? undefined : renderScope(tool.scope)
 	if (scope !== undefined) sections.push(scope)
 	if (tool.notices !== undefined && tool.notices.length > 0) {

apps/ai/src/mcp/tools/registry.ts

@@ -82,7 +82,7 @@ import {
 	type McpToolResult,
 } from "./types"
 import {
-	argumentNotices,
+	argumentWarnings,
 	enumValues,
 	formatDecodeFailure,
 	normalizeArguments,
@@ -530,7 +530,7 @@ const isValidNextCall = (call: NextCall, surface: McpToolSurface): boolean => {
 const finish = Effect.fnUntraced(function* (
 	definition: MapleToolDefinition,
 	run: ToolRun,
-	notices: ReadonlyArray<string>,
+	warnings: ReadonlyArray<string>,
 	surface: McpToolSurface,
 ) {
 	const invalid = nextCallsOf(run.doc).filter((call) => !isValidNextCall(call, surface))
@@ -544,7 +544,7 @@ const finish = Effect.fnUntraced(function* (
 	}
 	const doc = invalid.length === 0 ? run.doc : filterNextCalls(run.doc, (call) => !invalid.includes(call))
 	const text = renderToolDocWithinBudget(
-		notices.length === 0 ? doc : { ...doc, notices: [...notices, ...(doc.notices ?? [])] },
+		warnings.length === 0 ? doc : { ...doc, warnings: [...warnings, ...(doc.warnings ?? [])] },
 	)
 	const result: McpToolResult = {
 		content: [{ type: "text", text }],
@@ -607,7 +607,7 @@ export const executeRegisteredMcpToolUnscoped = Effect.fn("McpToolRegistry.execu
 	return yield* finish(
 		definition,
 		{ ...run, doc: windowNotes.decorate(run.doc) },
-		argumentNotices(normalized, definition.name),
+		argumentWarnings(normalized, definition.name),
 		surface,
 	)
 })

apps/ai/src/mcp/tools/tool-output.ts

@@ -212,7 +212,7 @@ const formatNumberOfChars = (chars: number): string =>
 	chars >= 1000 ? `${Math.round(chars / 1000)}k` : String(chars)
 
 /**
- * Render a doc within {@link MAX_TOOL_TEXT_CHARS}. Head (title, scope, notices) and tail (paging,
+ * Render a doc within {@link MAX_TOOL_TEXT_CHARS}. Head (title, warnings, scope, notices) and tail (paging,
  * next calls) always survive; body blocks are kept in order and the first that overflows is cut on a
  * row or line boundary, followed by a notice telling the model how to narrow the call.
  */