feat: add source-type filter to search (#246)#268
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR aims to expose the existing SearchOptions.source filter end-to-end via MCP, the CLI, and the REST API so callers can restrict results by document “source type”.
Changes:
- MCP
search-docs: adds an optionalsourceparameter and forwards it intosearchDocuments. - CLI
search: adds a--source <type>flag and forwards it intosearchDocuments. - REST API
GET /api/v1/search: adds asourcequery parameter and passes it intosearchDocuments. - CLI reference docs: documents the new flag.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/mcp/server.ts | Adds source to MCP search-docs tool schema and passes it through to core search. |
| src/cli/index.ts | Adds --source flag to libscope search and forwards it to core search. |
| src/api/routes.ts | Parses source from query params for /api/v1/search and forwards it to core search. |
| docs/reference/cli.md | Documents the new CLI --source flag. |
src/cli/index.ts
Outdated
| .description("Search indexed documents") | ||
| .option("--topic <topicId>", "Filter by topic") | ||
| .option("--library <name>", "Filter by library") | ||
| .option("--source <type>", "Filter by source type (e.g., 'confluence', 'url', 'topic', 'file')") |
There was a problem hiding this comment.
The option help text suggests values like confluence, url, and file, but documents.source_type is currently limited to library | topic | manual | model-generated by a DB CHECK constraint (src/db/schema.ts:20). With the current schema/indexing, those documented values will never match. Update the help text (and ideally validate the flag) to reflect the actual supported values, or extend the schema/indexing to support connector-origin source types.
| @@ -151,7 +152,14 @@ export async function handleRequest( | |||
| const offset = Number.isNaN(offsetParsed) ? undefined : offsetParsed; | |||
| const tags = tag ? [tag] : undefined; | |||
|
|
|||
| const result = await searchDocuments(db, provider, { query: q, topic, tags, limit, offset }); | |||
| const result = await searchDocuments(db, provider, { | |||
| query: q, | |||
| topic, | |||
| tags, | |||
| source, | |||
| limit, | |||
| offset, | |||
| }); | |||
There was a problem hiding this comment.
source is accepted as an arbitrary string and forwarded to searchDocuments, but the DB constrains documents.source_type to a small set of values (library, topic, manual, model-generated). For invalid values, the API will silently return 0 results, which is hard to diagnose. Consider validating source against the allowed set and returning a 400 VALIDATION_ERROR with a clear message when it’s not recognized.
| const result = await searchDocuments(db, provider, { | ||
| query: q, | ||
| topic, | ||
| tags, | ||
| source, | ||
| limit, | ||
| offset, | ||
| }); |
There was a problem hiding this comment.
This adds a new /api/v1/search query parameter (source) but there’s no test coverage asserting it is parsed and applied (there are already tests for limit and offset in tests/unit/api.test.ts). Add a unit test that indexes docs with different sourceType values and verifies ?source=manual / ?source=topic filters results correctly (and covers the invalid-value path if you add validation).
docs/reference/cli.md
Outdated
| |--------|-------------| | ||
| | `--library <name>` | Filter by library | | ||
| | `--topic <name>` | Filter by topic | | ||
| | `--source <type>` | Filter by source type (e.g., `confluence`, `url`, `topic`, `file`) | |
There was a problem hiding this comment.
The CLI docs advertise --source values like confluence, url, and file, but source_type is currently constrained to library | topic | manual | model-generated (see src/db/schema.ts:20), so those examples won’t work. Also, the CLI already uses --source-type elsewhere (docs list), so the docs should match whichever flag name you standardize on.
| | `--source <type>` | Filter by source type (e.g., `confluence`, `url`, `topic`, `file`) | | |
| | `--source-type <type>` | Filter by source type (e.g., `library`, `topic`, `manual`, `model-generated`) | |
src/mcp/server.ts
Outdated
| source: z | ||
| .string() | ||
| .optional() | ||
| .describe("Filter by source type (e.g., 'confluence', 'url', 'topic', 'file')"), |
There was a problem hiding this comment.
The new source filter is documented as accepting values like confluence, url, file, etc., but documents.source_type is currently constrained to ('library','topic','manual','model-generated') (see src/db/schema.ts:20). As written, passing the documented values will never match and will return empty results. Either (a) change this parameter to an enum of the actually-supported source_type values and update the description accordingly, or (b) add a schema migration + indexing changes to support connector-origin source types end-to-end.
src/cli/index.ts
Outdated
| .description("Search indexed documents") | ||
| .option("--topic <topicId>", "Filter by topic") | ||
| .option("--library <name>", "Filter by library") | ||
| .option("--source <type>", "Filter by source type (e.g., 'confluence', 'url', 'topic', 'file')") |
There was a problem hiding this comment.
CLI already uses --source-type for docs list (mapped to opts.sourceType), but search introduces a new --source flag for the same concept. This inconsistency makes the CLI harder to learn/script. Consider renaming this new option to --source-type (and keep passing it into searchDocuments as options.source).
Closes #246
Wires the existing
sourcefilter in SearchOptions to MCP, CLI, and REST API:search-docstool: newsourceparameter--source <type>flag on search commandsourcequery parameter on/api/v1/searchSupported source types:
confluence,url,topic,file, etc.