gabotorresruiz commented on code in PR #44682:
URL: https://github.com/apache/superset/pull/44682#discussion_r4147529446
##########
superset/mcp_service/server.py:
##########
@@ -681,6 +681,27 @@ async def _get_visible_tools(self, ctx: Context) ->
Sequence[Any]:
tools = await super()._get_visible_tools(ctx)
return _filter_tools_by_current_user_permission(tools)
+ async def _search(self, tools: Sequence[Tool], query: str) ->
Sequence[Tool]:
Review Comment:
Not a blocker, and I know the PR is deliberately scoped to `bm25`: does the
`regex` strategy want the same treatment? `RegexSearchTransform._search` takes
the first `max_results` text matches in catalog order with no ranking at all,
so a tool whose name shows up in enough sibling descriptions can get pushed out
the same way. I ran this branch with `strategy: "regex"` and nothing actually
falls off today, `update_dashboard` is just the last of 4 matches for its own
name, so it is cheap insurance rather than a live bug. Worth lifting this block
somewhere both `_FixedBM25SearchTransform` and `_FixedRegexSearchTransform` can
share it, or is `regex` legacy enough to leave alone?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
