Skip to content

fix(mcp): re-land native tool preservation with typed annotations#30645

Open
ayushh0110 wants to merge 2 commits into
BerriAI:litellm_internal_stagingfrom
ayushh0110:fix/mcp-filter-typed-annotations
Open

fix(mcp): re-land native tool preservation with typed annotations#30645
ayushh0110 wants to merge 2 commits into
BerriAI:litellm_internal_stagingfrom
ayushh0110:fix/mcp-filter-typed-annotations

Conversation

@ayushh0110

@ayushh0110 ayushh0110 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Relevant issues

Fixes #26212
Re-lands #26650 (reverted in #30637 due to Any-discipline CI check)

Pre-Submission checklist

  • I have added meaningful tests
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Type

Bug Fix

Changes

Same fix as #26650 with all type annotations tightened per the revert feedback. List[Any], bare set, and Any-typed parameters replaced with object, list[object], set[int], and set[str] to stay under the any-discipline ceiling.

The underlying bug: when mcp_semantic_tool_filter is enabled and a request has both MCP and native tools, native tools get silently dropped. In the expansion path, _expand_mcp_tools discards non-MCP tools since _parse_mcp_tools only returns MCP references. In the filtering path, filter_tools only knows about MCP-registered tools, so native tools get no similarity score and are removed.

The fix partitions tools into MCP-registered and native before filtering, applies semantic filtering only to MCP tools, then merges results back preserving original order. The expansion path now preserves native tools before calling _expand_mcp_tools and concatenates them back after. _emit_filter_metadata is extracted with its own try/except so a metadata error can't cause the outer handler to return None after data["tools"] was already mutated. async_post_call_response_headers_hook uses .get() instead of bare dict access to prevent KeyError on requests that skipped filtering

@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.76923% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/proxy/hooks/mcp_semantic_filter/hook.py 80.76% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the bug where native (non-MCP) tools were silently dropped when mcp_semantic_tool_filter was enabled alongside a mixed tool list. The core change partitions the request's tools into MCP-registered and native subsets, applies semantic filtering only to the MCP subset, then reconstructs the list in original order — guaranteeing native tools are never scored or removed by the filter.

  • _is_mcp_tool classifies each tool by shape-first (Chat Completions and Responses API dicts are always native) then registry lookup, while _emit_filter_metadata is extracted with its own try/except so a metadata error can't clobber the already-mutated data[\"tools\"].
  • The _should_expand_mcp_tools path now stashes native tools before expansion and concatenates them back, and async_post_call_response_headers_hook replaces bare dict access with .get() to prevent KeyError on requests that bypassed filtering.
  • Four new mock-only regression tests validate mixed MCP+native, all-native pass-through, Responses-API name collision, and original-order preservation.

Confidence Score: 5/5

The change is safe to merge: it repairs a clear drop-silent bug without altering any external API contract, and all new paths have direct mock-based test coverage.

The partitioning and reconstruction logic is internally consistent — _is_mcp_tool uses _extract_tool_info and the same method is used again in the reconstruction loop, so tool names are always compared against the same key space. The _emit_filter_metadata extraction prevents a metadata error from masking a mutated tools list. The .get() guard closes a real KeyError. No previously passing assertions were weakened, and the four new tests cover the exact scenarios described in the bug report.

No files require special attention.

Important Files Changed

Filename Overview
litellm/proxy/hooks/mcp_semantic_filter/hook.py Adds native-tool partitioning logic: _is_mcp_tool, _emit_filter_metadata, semantic-filter-only-for-MCP path, order-preserving tool reconstruction, and a .get() guard in the headers hook — all correct and well-bounded.
tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py Adds four targeted regression tests covering mixed MCP+native, all-native, Responses-API name collision, and original-order preservation — all mock-based, no real network calls.

Reviews (2): Last reviewed commit: "fix(mcp): tighten _is_mcp_tool Chat Comp..." | Re-trigger Greptile

Comment thread litellm/proxy/hooks/mcp_semantic_filter/hook.py Outdated
Comment thread litellm/proxy/hooks/mcp_semantic_filter/hook.py
@Sameerlite

Copy link
Copy Markdown
Collaborator

Thanks for the PR! A couple of things to get this over the finish line:

  • Greptile's code review came back with a score below 5/5 — could you take a look at its feedback and address the comments? Once it reaches 5/5 with no open threads we'll take another look.
  • Could you add proof of the change working (screenshots, test output, or a sample request/response)? Even a quick curl before/after really speeds up the review.

Once those are addressed we'll take another look — appreciate the contribution!

@ayushh0110 ayushh0110 force-pushed the fix/mcp-filter-typed-annotations branch from 5d0d375 to 8421d01 Compare June 19, 2026 16:36
@ayushh0110

Copy link
Copy Markdown
Contributor Author

@greptileai

@ayushh0110

Copy link
Copy Markdown
Contributor Author

Hii @Sameerlite

Addressed both items:

  1. Greptile is now 5/5 after tightening the _is_mcp_tool shape check (8421d01). Both P2 threads resolved.

  2. Proof of fix against a live proxy (localhost:4000) with mcp_semantic_tool_filter enabled:

Test 1 - Single native tool + tool_choice=auto (exact #26212 repro): Status 200, native tool preserved, LLM calls weather_lookup correctly. No spurious x-litellm-semantic-filter header.

Test 2 - Multiple native tools: Both weather_lookup and stock_price preserved and called.

Test 3 - No tools: Plain text response, no filter interference.

Test 4 - Unit tests: 20/20 pass.

image image image image image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: mcp_semantic_tool_filter silently drops all non-MCP ("native") tools

2 participants