feat(search): match server description as well as name#1471
Open
amitvijapur wants to merge 1 commit into
Open
Conversation
The ?search= query on GET /v0/servers only matched server_name, so servers were undiscoverable by what they do unless the search term happened to appear in the name too (modelcontextprotocol#135 added the name-only version of this). Add SubstringDescription to ServerFilter and have the handler set it alongside SubstringName from the same search term. buildFilterConditions combines the two into a single OR'd condition (server_name ILIKE ... OR value ->> 'description' ILIKE ...) rather than the AND every other filter pair uses, since requiring both would silently break the description half of the search. The description column lives inside the JSONB value blob (there's no dedicated column, unlike server_name), so the condition reads it via value ->> 'description'. Fixes modelcontextprotocol#1453.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1453
Change
Adds
SubstringDescriptiontoServerFilter; the?search=handler now sets it alongsideSubstringNamefrom the same term, so search matches server name or description.Two implementation notes where this deviates from the issue's literal proposal, deliberately:
buildFilterConditions' outer AND-join require a match on both name and description. When both fields are set, they're combined into a single(server_name ILIKE ... OR value ->> 'description' ILIKE ...)condition instead. A white-box test asserts the generated SQL to lock this in.descriptionhas no dedicated column (unlikeserver_name, materialized in migration 009) — it lives inside the JSONBvalueblob, so the condition readsvalue ->> 'description'.The existing LIKE-metacharacter escaping is preserved via a shared
likePatternhelper (same escaping order, sameESCAPE '\'pairing), so?search=%still can't wildcard-match everything through the new description path either.Scoped out: the issue also suggests ranking name matches above description matches.
ListServersuses keyset/cursor pagination ordered by(server_name, version), and reordering by match-type would break that pagination scheme, so relevance ranking is left out of this PR rather than silently half-done. Happy to explore it separately if there's an agreed approach.Tests
TestBuildFilterConditions_SearchMatchesNameOrDescription(6 subtests, no DB required) asserting the generated SQL/args, including the OR-not-AND behavior.postgres_test.goandservers_test.go(description-only match, combined search, no-match). Honest note: these compile cleanly (go test -run '^$') but I could not execute them locally because port 5432 is occupied by an unrelated service on my machine and the test harness's connection string is fixed tolocalhost:5432— CI's Postgres job will exercise them.go build ./...,go vet ./...,gofmtclean;golangci-lint runreports no new findings versusmainon the touched packages.