fix(ai): warn first when an MCP call's argument was ignored by Makisuo · Pull Request #1386 · MapleTechLabs/maple

Makisuo · 2026-10-10T23:17:04Z

Problem

An agent called list_services with {"time_range":"today"}. The tool takes start_time/end_time (default 6 hours), so the key was dropped and the default window ran. The registry did report it, but the report was easy to miss and half-wrong:

## Services

Time range: 2026-10-10 17:15:10 to 2026-10-10 23:15:10 · Now (UTC): ...

Note: `time_range` is not a parameter of `list_services` and was ignored. Did you mean `start_time`?

It came after the scope line, as a Note: alongside informational notes (clamped windows).
It never said the result was computed without the key, so the agent read the default window as its answer.
time_range names a whole window, but the edit-distance match suggested only start_time.

Change

ToolDoc gains warnings, rendered directly under the title as Warning: lines. The registry puts ignored-argument reports there for every tool; notices stays for informational notes.
The warning says the result is for the call without the key.
A key naming a whole window (time_range, lookback, period, since, ...) on a tool that takes both bounds suggests start_time/end_time. A single-bound key (start) still suggests the one bound. The decode-failure message uses the same suggestions.

Now:

## Services

Warning: `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`?

Time range: ...

Not done: a `time_range` alias on `P.timeWindow`

Aliases rename a key and keep the value, so time_range: "today" would land in start_time and fail to decode as a timestamp. time_range was never a parameter name for these tools, so it is not a retired name. It already means something else on create_dashboard (the dashboard's saved relative range). Accepting relative windows would be a new parameter, not an alias, so it does not fit "one spelling per concept".

Tests

decode-issues.test.ts: window keys suggest both bounds, single bounds stay single, no window suggestion on tools without one, warning text.
tool-doc.test.ts: warnings render before scope, notices after.
tool-contract.test.ts: list_services with time_range succeeds and the warning is the first thing under the title.

Ran those three plus registry.contract.test.ts and @maple/ai typecheck. The full suite and repo-wide typecheck were not run.

🤖 Generated with Claude Code

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by CodeRabbit

New Features
- Tool results now warn when an unrecognized parameter is ignored and clarify that the result reflects a call made without it.
- Warnings can suggest the correct parameter names, including both start and end time parameters when a time-range parameter is supplied.
- Warnings appear directly below the tool result title, while other notices remain separate.
- Important warnings remain visible when tool results are shortened to fit display limits.

An unknown key was reported as a Note below the scope line, suggested only start_time for time_range, and never said the result ignored it. Ignored keys are now a Warning directly under the title that says the result is for the call without the key, and a window key points at both start_time/end_time.

maple-review-bot · 2026-10-10T23:17:16Z

Maple review

🔴 Confidence 4/10 · risky as written
Held at 4 because a critical finding is open.
_{quality 75/100 · 1 critical · tests covered · risk medium}

Moves ignored-argument reports from notices to a new ToolDoc.warnings rendered under the title, and makes a window-named unknown key suggest both start_time and end_time. The rendering change itself is sound, but the rename left a caller broken. The registry-stack error issues listed in the kickoff are tool failures unrelated to this rendering path, and the diff does not address them.

ToolDoc.warnings renders Warning: lines under the title, before scope
argumentWarnings replaces argumentNotices and says the result is for the call without the key
Window-named unknown keys suggest both start_time and end_time

Findings

🔴 Critical · F1 · argumentNotices rename leaves pr-review-local.ts importing a missing export

correctness · apps/ai/src/mcp/lib/decode-issues.ts:171-175

argumentNotices is gone from this module, but apps/ai/scripts/pr-review-local.ts:61 still does import { normalizeArguments, argumentNotices } from "@/mcp/lib/decode-issues" and calls it at line 769, so the local PR-review runner fails at import. apps/ai/src/chat/pr-review-replay.test.ts imports makeExecutor from that script, so bun run test in @maple/ai (vitest include: ["src/**/*.test.ts"]) fails too. This slipped CI because apps/ai/tsconfig.json includes only src/** and excludes *.test.ts.

Update both references to `argumentWarnings` in `apps/ai/scripts/pr-review-local.ts` (the import on line 61 and the call on line 769).

🤖 Prompt to fix this finding with an AI agent

Findings from an automated review of commit 162a1d90bf29a5e618b69d7bc3c7568ef4b139a6. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F1 · Critical · correctness · apps/ai/src/mcp/lib/decode-issues.ts:171-175
`argumentNotices` rename leaves `pr-review-local.ts` importing a missing export
`argumentNotices` is gone from this module, but `apps/ai/scripts/pr-review-local.ts:61` still does `import { normalizeArguments, argumentNotices } from "@/mcp/lib/decode-issues"` and calls it at line 769, so the local PR-review runner fails at import. `apps/ai/src/chat/pr-review-replay.test.ts` imports `makeExecutor` from that script, so `bun run test` in `@maple/ai` (vitest `include: ["src/**/*.test.ts"]`) fails too. This slipped CI because `apps/ai/tsconfig.json` includes only `src/**` and excludes `*.test.ts`.
Suggested fix: Update both references to `argumentWarnings` in `apps/ai/scripts/pr-review-local.ts` (the import on line 61 and the call on line 769).

Production impact

Open errors in the changed files

Issue	Service	Occurrences	File
@maple/api/vcs/VcsSourceRepositoryNotFoundError	maple-ai	68	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	42	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	36	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	18	`apps/ai/src/mcp/tools/registry.ts`
@maple/api/vcs/GithubAppError	maple-chat	14	`apps/ai/src/mcp/tools/registry.ts`
@maple/http/errors/IntegrationsUpstreamError	maple-chat	14	`apps/ai/src/mcp/tools/registry.ts`
@maple/http/errors/IntegrationsUpstreamError	maple-chat	11	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	9	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	9	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	8	`apps/ai/src/mcp/tools/registry.ts`

After this merges, Maple checks whether they stop.

What was checked

Window suggestion is gated on the tool actually declaring both bounds (decode-issues.ts:101)
Single-bound keys still fall back to suggestParameter (decode-issues.ts:104)
No other consumer of NormalizedArguments.unknown reads the old suggestion field

_{162a1d9 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.}

coderabbitai · 2026-10-10T23:17:17Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 6918dbf0-f0a4-48f3-9e26-09bafba95814

📥 Commits

Reviewing files that changed from the base of the PR and between 3459837 and 6b0f968.

📒 Files selected for processing (2)

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

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Walkthrough

Ignored arguments now produce warnings that describe omitted parameters and suggest matching time-bound parameters when available. Tool execution and local review output place these warnings before notices.

Changes

Ignored-argument warnings

Layer / File(s)	Summary
Suggestion and warning generation `apps/ai/src/mcp/lib/decode-issues.ts`, `apps/ai/src/mcp/lib/decode-issues.test.ts`	Unknown arguments now have a suggestions array. Window-like keys can suggest both time bounds. Warnings state that the result is for a call without the ignored key. Decode-failure output uses the same suggestion format.
Warning delivery and rendering `apps/ai/src/mcp/lib/tool-doc.ts`, `apps/ai/src/mcp/tools/registry.ts`, `apps/ai/src/mcp/tools/tool-output.ts`, `apps/ai/scripts/pr-review-local.ts`, `apps/ai/src/mcp/lib/tool-doc.test.ts`, `apps/ai/src/mcp/__evals__/tool-contract.test.ts`	The registry and local review runner use `argumentWarnings`. Tool documents render warnings after the title and before notices. Tests cover ignored-parameter warnings, time-bound suggestions, and the warehouse fixture fallback.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6b0f9

Ignored arguments are surfaced with parameter suggestions. The identified local-review import issue is fixed, and no actionable merge-blocking risk remains in the supplied evidence.

Pre-merge checks | 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: ignored MCP call arguments now generate warnings that appear first.
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 8…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

maple-review-bot

1 inline note from Maple's review. The score and summary are in the review comment above.

maple-review-bot · 2026-10-12T00:46:10Z

Maple review

🔴 Confidence 4/10 · risky as written
Held at 4 because a critical finding is open.
_{quality 65/100 · 1 critical · 1 warning · tests covered · risk low}

Moves the ignored-argument report out of notices into a new ToolDoc.warnings block rendered first under the title, and points whole-window keys at both time bounds. Safe to merge once the window heuristic stops overriding a closer match. The open McpQueryError / VCS / integration errors in registry.ts are query and upstream failures this diff does not touch.

ToolDoc.warnings renders as Warning: lines directly under the title, before scope
argumentNotices became argumentWarnings, and every call site was renamed
normalizeArguments reports suggestions: string[] instead of one suggestion
Window-named keys (time_range, lookback) suggest start_time/end_time together

Findings

🟠 Warning · F2 · suggestParameters drops a closer match for any key containing a window word

correctness · apps/ai/src/mcp/lib/decode-issues.ts:100-106

The window branch returns start_time/end_time before consulting suggestParameter, and WINDOW_WORDS contains duration, last, ago and period. On search_traces, which takes both bounds and min_duration_ms/max_duration_ms, an agent sending min_duration or duration now reads "Did you mean start_time/end_time?" instead of min_duration_ms — the previous suggestParameter matched it as an abbreviation (score 1), so the model is told to drop its duration filter and re-run over a window.

Compute `suggestParameter` first and take the both-bounds branch only when it found nothing or found one of the bounds, so a key that abbreviates or shares a word with another parameter keeps its single suggestion.

🤖 Prompt to fix this finding with an AI agent

Findings from an automated review of commit 3459837cd06f5e7fe897cb68d47bd06d9046fbd6. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F2 · Warning · correctness · apps/ai/src/mcp/lib/decode-issues.ts:100-106
`suggestParameters` drops a closer match for any key containing a window word
The window branch returns `start_time`/`end_time` before consulting `suggestParameter`, and `WINDOW_WORDS` contains `duration`, `last`, `ago` and `period`. On `search_traces`, which takes both bounds and `min_duration_ms`/`max_duration_ms`, an agent sending `min_duration` or `duration` now reads "Did you mean `start_time`/`end_time`?" instead of `min_duration_ms` — the previous `suggestParameter` matched it as an abbreviation (score 1), so the model is told to drop its duration filter and re-run over a window.
Suggested fix: Compute `suggestParameter` first and take the both-bounds branch only when it found nothing or found one of the bounds, so a key that abbreviates or shares a word with another parameter keeps its single suggestion.

Still open from earlier reviews

🔴 Critical · F1 · argumentNotices rename leaves pr-review-local.ts importing a missing export · apps/ai/src/mcp/lib/decode-issues.ts:171

Production impact

Open errors in the changed files

Issue	Service	Occurrences	File
@maple/api/vcs/VcsSourceRepositoryNotFoundError	maple-ai	68	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	42	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	36	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	18	`apps/ai/src/mcp/tools/registry.ts`
@maple/api/vcs/GithubAppError	maple-chat	14	`apps/ai/src/mcp/tools/registry.ts`
@maple/http/errors/IntegrationsUpstreamError	maple-chat	14	`apps/ai/src/mcp/tools/registry.ts`
@maple/http/errors/IntegrationsUpstreamError	maple-chat	11	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	9	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	9	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	8	`apps/ai/src/mcp/tools/registry.ts`

After this merges, Maple checks whether they stop.

What was checked

pr-review-local.ts:61,769 now imports and calls argumentWarnings; F1's missing export is gone at this head
No other importer of argumentNotices or reader of unknown[].suggestion remains (evals/targets.ts reads only .length, registry.ts:486,590 only .key)
formatDecodeFailure keeps its old single-suggestion output for cmd/command (decode-issues.test.ts:55)

_{3459837 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.}

maple-review-bot

1 inline note from Maple's review. The score and summary are in the review comment above.

maple-review-bot · 2026-10-12T00:51:44Z

Maple review

🟢 Confidence 9/10 · safe to merge
Both earlier findings are demonstrably fixed in the two files that changed, and the window-suggestion logic is pinned by tests I re-derived by hand.
_{quality 100/100 · no findings · tests covered · risk medium}

Ignored-argument reports move from Note: into a new ToolDoc.warnings rendered first under the title, and a window-shaped unknown key now suggests both bounds. The two earlier findings are fixed; safe to merge.

argumentWarnings replaces argumentNotices and renders as Warning: above scope (tool-doc.ts:127)
UnknownArgument.suggestions carries both window bounds for a time_range-like key
A closer non-bound match now wins over the window guess (decode-issues.ts:105)

Fixed since the last review

✅ ~~F1 · argumentNotices rename leaves pr-review-local.ts importing a missing export~~
✅ ~~F2 · suggestParameters drops a closer match for any key containing a window word~~

Production impact

Open errors in the changed files

Issue	Service	Occurrences	File
@maple/api/vcs/VcsSourceRepositoryNotFoundError	maple-ai	68	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	42	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	36	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	18	`apps/ai/src/mcp/tools/registry.ts`
@maple/api/vcs/GithubAppError	maple-chat	14	`apps/ai/src/mcp/tools/registry.ts`
@maple/http/errors/IntegrationsUpstreamError	maple-chat	14	`apps/ai/src/mcp/tools/registry.ts`
@maple/http/errors/IntegrationsUpstreamError	maple-chat	11	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	9	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	9	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	8	`apps/ai/src/mcp/tools/registry.ts`

After this merges, Maple checks whether they stop.

What was checked

F1: pr-review-local.ts:61,769 imports and calls argumentWarnings, so the missing export is gone
F2: min_duration on a tool with min_duration_ms returns only min_duration_ms (decode-issues.ts:107)
Every notices/warnings consumer audited: registry.ts:547 merges, windowNotes.decorate keeps its own notices

_{6b0f968 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.}

maple-review-bot Bot reviewed Oct 10, 2026

View reviewed changes

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

fix(ai): use argumentWarnings in the local PR review script

3459837

maple-review-bot Bot reviewed Oct 12, 2026

View reviewed changes

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

fix(ai): keep a closer non-bound match over the window suggestion

6b0f968

Navigation Menu

Repository navigation

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

fix(ai): warn first when an MCP call's argument was ignored#1386
Makisuo wants to merge 3 commits into
mainfrom
fix/mcp-unknown-param-notice

Makisuo commented Oct 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

maple-review-bot Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

maple-review-bot Bot left a comment

Uh oh!

Uh oh!

maple-review-bot Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

maple-review-bot Bot left a comment

Uh oh!

Uh oh!

maple-review-bot Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Makisuo commented Oct 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Not done: a time_range alias on P.timeWindow

Tests

Summary by CodeRabbit

Uh oh!

maple-review-bot Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Maple review

Findings

Production impact

Uh oh!

coderabbitai Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

maple-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maple-review-bot Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Maple review

Findings

Still open from earlier reviews

Production impact

Uh oh!

maple-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maple-review-bot Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Maple review

Fixed since the last review

Production impact

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Makisuo commented Oct 10, 2026 •

edited by coderabbitai Bot

Loading

Not done: a `time_range` alias on `P.timeWindow`

maple-review-bot Bot commented Oct 10, 2026 •

edited

Loading

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

maple-review-bot Bot commented Oct 12, 2026 •

edited

Loading

maple-review-bot Bot commented Oct 12, 2026 •

edited

Loading