feat: add public browse/search MCP discovery layer for Codex-facing workflows#33
Conversation
Original prompt from Stanislau
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 43 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds a public discovery layer: new browse/search MCP tools and Zod contracts, runtime browse/search implementations with snippet scoring, new discovery types, tool-family reorganization (public/expert/admin), server description update, and tests/documentation updates covering the new APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BrowseTool as browse_fpf_catalog Tool
participant Runtime as FpfRuntime
participant Snapshot
participant Filter
Client->>BrowseTool: Call with filters (kind, part, status, limit, forceRefresh)
BrowseTool->>Runtime: browse(options)
Runtime->>Runtime: refreshSnapshot(forceRefresh)
Runtime->>Snapshot: Load compiled nodes
Snapshot-->>Runtime: compiledNodes[]
loop For each compiledNode
Runtime->>Filter: nodeToCatalogEntry(node, snapshot)
Filter-->>Runtime: CatalogEntry
Runtime->>Runtime: Apply kind/part/status filters
end
Runtime->>Runtime: Sort entries by id, apply limit
Runtime-->>BrowseTool: BrowseCatalogResult {entries, total, filters, snapshot}
BrowseTool-->>Client: Return browse results
sequenceDiagram
participant Client
participant SearchTool as search_fpf Tool
participant Runtime as FpfRuntime
participant Snapshot
participant Tokenizer as TextUtils
participant Scorer as ScoringEngine
Client->>SearchTool: Call with query (query, kind?, limit, forceRefresh)
SearchTool->>Runtime: search(query, options)
Runtime->>Runtime: refreshSnapshot(forceRefresh)
Runtime->>Snapshot: Load compiled nodes
Snapshot-->>Runtime: compiledNodes[]
Runtime->>Tokenizer: normalize/tokenize(query)
Tokenizer-->>Runtime: queryTokens[]
loop For each compiledNode
Runtime->>Scorer: scoreOverlap(queryTokens, node.searchableText)
Scorer-->>Runtime: score
alt score > 0
Runtime->>Runtime: extractSnippet(node, queryTokens)
Runtime->>Runtime: Build SearchHit {id, kind, title, score, snippet}
end
end
Runtime->>Runtime: Sort hits by score DESC then id, apply limit
Runtime-->>SearchTool: SearchResult {query, hits, total, snapshot}
SearchTool-->>Client: Return search results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a discovery layer to the FPF MCP server by adding browse_fpf_catalog and search_fpf tools. These additions include new Zod schemas, TypeScript interfaces, and runtime logic for filtering and full-text searching across compiled nodes. The tools have also been reorganized into public, expert, and admin categories. Feedback suggests adding a limit parameter to the browse tool to handle large catalogs, optimizing the runtime filtering logic to improve performance, and refining the snippet extraction to ensure words are not cut in half.
| export const browseFpfCatalogInputSchema = z | ||
| .object({ | ||
| part: z.string().optional(), | ||
| status: z.string().optional(), | ||
| kind: nodeKindSchema.optional(), | ||
| forceRefresh: z.boolean().optional(), | ||
| }) | ||
| .strict(); |
There was a problem hiding this comment.
The browse_fpf_catalog tool should ideally include a limit parameter (similar to the search_fpf tool) to prevent returning an excessively large number of entries in a single response. As the specification grows, returning the entire catalog could impact performance or exceed MCP message size limits.
There was a problem hiding this comment.
Addressed — added limit parameter to browseFpfCatalogInputSchema (min 1, max 500) and the runtime browse() method now defaults to 200 entries. The total field still reports the full count of matched entries so callers know when they're seeing a subset.
Also applied filter-before-map optimization per the suggestion below — nodes are now filtered before calling nodeToCatalogEntry() to avoid unnecessary graph/lexicon lookups.
| let entries: CatalogEntry[] = Object.values(snapshot.compiledNodes).map((node) => | ||
| nodeToCatalogEntry(node, snapshot), | ||
| ); | ||
|
|
||
| if (options.kind) { | ||
| entries = entries.filter((e) => e.kind === options.kind); | ||
| } | ||
| if (options.part) { | ||
| const partLower = options.part.toLowerCase(); | ||
| entries = entries.filter((e) => e.part?.toLowerCase() === partLower); | ||
| } | ||
| if (options.status) { | ||
| const statusLower = options.status.toLowerCase(); | ||
| entries = entries.filter((e) => e.status?.toLowerCase() === statusLower); | ||
| } |
There was a problem hiding this comment.
Performance can be improved by filtering the nodes before mapping them to catalog entries. This avoids unnecessary lookups in the graph and lexicon for nodes that are ultimately excluded by the filters.
| let entries: CatalogEntry[] = Object.values(snapshot.compiledNodes).map((node) => | |
| nodeToCatalogEntry(node, snapshot), | |
| ); | |
| if (options.kind) { | |
| entries = entries.filter((e) => e.kind === options.kind); | |
| } | |
| if (options.part) { | |
| const partLower = options.part.toLowerCase(); | |
| entries = entries.filter((e) => e.part?.toLowerCase() === partLower); | |
| } | |
| if (options.status) { | |
| const statusLower = options.status.toLowerCase(); | |
| entries = entries.filter((e) => e.status?.toLowerCase() === statusLower); | |
| } | |
| const partLower = options.part?.toLowerCase(); | |
| const statusLower = options.status?.toLowerCase(); | |
| const entries = Object.values(snapshot.compiledNodes) | |
| .filter((node) => { | |
| if (options.kind && node.kind !== options.kind) return false; | |
| if (partLower && node.part?.toLowerCase() !== partLower) return false; | |
| if (statusLower && node.status?.toLowerCase() !== statusLower) return false; | |
| return true; | |
| }) | |
| .map((node) => nodeToCatalogEntry(node, snapshot)); |
There was a problem hiding this comment.
Addressed — applied the filter-before-map pattern as suggested. Nodes are now filtered by kind/part/status before calling nodeToCatalogEntry(), avoiding unnecessary graph and lexicon lookups for excluded nodes.
| const start = Math.max(0, bestPos - SNIPPET_RADIUS); | ||
| const end = Math.min(searchableText.length, bestPos + bestLen + SNIPPET_RADIUS); | ||
| let snippet = searchableText.slice(start, end).replace(/\s+/g, ' ').trim(); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Addressed — snippet boundaries are now snapped to the nearest whitespace so words aren't cut in half at either end.
There was a problem hiding this comment.
Pull request overview
Adds a public “discovery” layer to the MCP surface so agent operators can first browse/search the compiled FPF index before drilling into specific nodes (patterns/routes/lexicon).
Changes:
- Introduces discovery-layer runtime APIs (
browse,search) and corresponding result types. - Adds MCP tool contracts + tool definitions for
browse_fpf_catalogandsearch_fpf, and reorganizes tools into public/expert/admin families. - Updates MCP server description and tool list expectations in tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/mcp-server.test.ts | Updates expected tool lists/order for full vs public MCP surfaces. |
| src/runtime/types.ts | Adds discovery-layer TypeScript interfaces for catalog/search results. |
| src/runtime/runtime.ts | Implements browse() and search() on FpfRuntime, including snippet generation. |
| src/mcp/tool-contracts.ts | Adds Zod schemas for new browse/search tools. |
| src/mcp/tools.ts | Registers new tools and reorganizes tool families (public/expert/admin). |
| src/mastra/mcp/server.ts | Updates the public server description to reflect discovery tools. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function extractSnippet(searchableText: string, queryTokens: string[]): string { | ||
| const lower = searchableText.toLowerCase(); | ||
| let bestPos = 0; | ||
| let bestLen = 0; | ||
|
|
||
| for (const token of queryTokens) { | ||
| const pos = lower.indexOf(token); | ||
| if (pos !== -1 && token.length > bestLen) { | ||
| bestPos = pos; | ||
| bestLen = token.length; |
There was a problem hiding this comment.
extractSnippet() looks for queryTokens via lower.indexOf(token), but tokenize() generates collapsed tokens for dotted IDs (e.g., A.2.3 -> a23). Those collapsed tokens can contribute to a positive scoreOverlap() yet never appear as a literal substring in searchableText, causing the snippet to default to the start of the node text even when the match is elsewhere. Consider centering snippets using raw ID matches (e.g., extractIds(query) searched against searchableText), or otherwise keeping a non-collapsed token variant for snippet positioning so ID-based searches produce correctly centered excerpts.
| function extractSnippet(searchableText: string, queryTokens: string[]): string { | |
| const lower = searchableText.toLowerCase(); | |
| let bestPos = 0; | |
| let bestLen = 0; | |
| for (const token of queryTokens) { | |
| const pos = lower.indexOf(token); | |
| if (pos !== -1 && token.length > bestLen) { | |
| bestPos = pos; | |
| bestLen = token.length; | |
| function findSnippetTokenMatch( | |
| searchableText: string, | |
| lower: string, | |
| token: string, | |
| ): { pos: number; len: number } | null { | |
| const literalPos = lower.indexOf(token); | |
| if (literalPos !== -1) { | |
| return { pos: literalPos, len: token.length }; | |
| } | |
| if (token.length === 0) { | |
| return null; | |
| } | |
| const escapedChars = Array.from(token).map((char) => | |
| char.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), | |
| ); | |
| const separator = '[^a-z0-9]*'; | |
| const collapsedPattern = escapedChars.join(separator); | |
| const match = new RegExp(collapsedPattern, 'i').exec(searchableText); | |
| if (!match || match.index === undefined) { | |
| return null; | |
| } | |
| return { pos: match.index, len: match[0].length }; | |
| } | |
| function extractSnippet(searchableText: string, queryTokens: string[]): string { | |
| const lower = searchableText.toLowerCase(); | |
| let bestPos = 0; | |
| let bestLen = 0; | |
| for (const token of queryTokens) { | |
| const match = findSnippetTokenMatch(searchableText, lower, token); | |
| if (match !== null && token.length > bestLen) { | |
| bestPos = match.pos; | |
| bestLen = match.len; |
There was a problem hiding this comment.
Addressed — added findTokenPosition() helper that first tries a literal substring match, then falls back to a regex pattern with [^a-z0-9]* separators between characters. This handles collapsed tokens like "a23" from "A.2.3" so snippets center correctly on dotted IDs.
| async browse( | ||
| options: { part?: string; status?: string; kind?: NodeKind; forceRefresh?: boolean } = {}, | ||
| ): Promise<BrowseCatalogResult> { | ||
| await this.refresh(options.forceRefresh ?? false); | ||
| const snapshot = await this.requireSnapshot(); | ||
|
|
||
| let entries: CatalogEntry[] = Object.values(snapshot.compiledNodes).map((node) => | ||
| nodeToCatalogEntry(node, snapshot), | ||
| ); | ||
|
|
||
| if (options.kind) { | ||
| entries = entries.filter((e) => e.kind === options.kind); | ||
| } | ||
| if (options.part) { | ||
| const partLower = options.part.toLowerCase(); | ||
| entries = entries.filter((e) => e.part?.toLowerCase() === partLower); | ||
| } | ||
| if (options.status) { | ||
| const statusLower = options.status.toLowerCase(); | ||
| entries = entries.filter((e) => e.status?.toLowerCase() === statusLower); | ||
| } | ||
|
|
||
| entries.sort((a, b) => a.id.localeCompare(b.id)); | ||
|
|
||
| return { | ||
| entries, | ||
| total: entries.length, | ||
| filters: { | ||
| part: options.part, | ||
| status: options.status, | ||
| kind: options.kind, | ||
| }, | ||
| snapshot: { sourceHash: snapshot.sourceHash, builtAt: snapshot.builtAt }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
New browse() / search() discovery APIs are introduced here, but there don’t appear to be runtime- or MCP-level tests asserting their behavior (filters, ordering, scoring/limit, snippet output, snapshot metadata). Since FpfRuntime already has test coverage elsewhere, adding focused tests for these new methods/tools would help prevent regressions and lock in the intended discovery workflow semantics.
There was a problem hiding this comment.
Added 13 focused discovery-layer tests in tests/discovery-layer.test.ts covering:
browse(): unfiltered results, sort order, kind/part/status filters, limit, descriptionssearch(): ranked hits, score ordering, snippets, kind filter, limit, zero-hit case, dotted-ID snippet centering
All passing alongside the existing 39 tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 478c9962cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let entries: CatalogEntry[] = Object.values(snapshot.compiledNodes).map((node) => | ||
| nodeToCatalogEntry(node, snapshot), | ||
| ); |
There was a problem hiding this comment.
Limit browse_fpf_catalog response size
browse() materializes and returns every compiled node when no filters are provided, so a default call now emits the full catalog payload. In this repo snapshot that is thousands of entries (~MB-sized JSON), which is large enough to hit MCP/tooling payload limits or consume most model context before the user can do any follow-up discovery. This makes the new default discovery path unreliable in real client sessions unless paging or a default limit is added.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed — browse() now has a limit parameter (default 200, max 500) enforced both in the Zod schema (z.number().int().min(1).max(500).optional()) and the runtime method (Math.min(options.limit ?? 200, 500)). The total field still reports the full count of matched entries so callers know when they're seeing a subset. Additionally, nodes are filtered by kind/part/status before mapping to catalog entries, so the overhead is proportional to matched nodes rather than the full catalog.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3e8e8614e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const normalizedQuery = normalizeForLookup(query); | ||
| const queryTokens = tokenize(normalizedQuery); |
There was a problem hiding this comment.
Tokenize query before lowercasing it
search() lowercases the query via normalizeForLookup() before calling tokenize(), but tokenize() depends on uppercase transitions (([a-z])([A-Z])) to split camel-case terms. This makes searches like U.BoundedContext tokenize to boundedcontext/uboundedcontext instead of bounded + context, so scoreOverlap() can be zero even when the target node text contains that concept, causing real search misses for canonical FPF names.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed. tokenize() already does its own camelCase splitting and lowercasing internally, so pre-lowercasing via normalizeForLookup() was destroying the uppercase transitions needed for the split. Now we call tokenize(query) directly on the raw query string, which preserves splits like BoundedContext → bounded + context.
18dd0dd to
124daae
Compare
Code ReviewOverviewAdds two new public MCP tools — Strengths
Issues & Risks🔴 Conflicts with PR #36 hosted surfacePR #36 (already open) states the hosted public surface is Two concrete problems:
Please either:
|
124daae to
c7fc6eb
Compare
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
venikman
left a comment
There was a problem hiding this comment.
Two review findings:
src/runtime/runtime.ts:251-261— [P2] Default browse page hides routes and lexemes entirely
browse() mixes every compiled node into one list, sorts globally by ID, and only then applies the default 200-entry cap. On the current snapshot that first page contains 200 patterns and 0 routes / 0 lexemes, so the new public discovery surface never exposes those kinds unless the caller already knows to add a filter. That undercuts the stated goal of giving operators an obvious first-pass discovery path across patterns, routes, and lexicon entries.
src/runtime/runtime.ts:293-309— [P2] Exact-ID search does not rank the exact node first
search() relies only on scoreOverlap() and then breaks ties lexicographically by ID, so exact selector queries are not promoted above broader prefix matches. On the current snapshot, searching A.1.1 returns A.1 ahead of A.1.1 with the same score. For a discovery tool that is supposed to lead into opening the right node, exact ID and exact title hits need an explicit ranking boost before the generic overlap sort.
Add a public discovery layer for Codex-facing workflows: - browse_fpf_catalog: browse compiled patterns, routes, and lexicon entries with optional part/status/kind filters - search_fpf: full-text search across all compiled nodes with ranked hits and contextual snippets Reorganize tool families per issue #15 spec: - Public: browse, search, ask, query, read - Expert: inspect node, inspect anchor, expand citations, trace - Admin: index status, refresh index Closes #15 Co-Authored-By: Stanislau <nedbailov375426@gmail.com>
…ENTS.md, tests - Add limit param to browse_fpf_catalog (default 200, max 500) - Filter compiled nodes before mapping to catalog entries (perf) - Snap snippet boundaries to word boundaries - Fix snippet centering for collapsed/dotted-ID tokens (e.g. A.2.3) - Update AGENTS.md to reflect new public/expert/admin surface - Add 13 focused discovery-layer tests (browse + search) Co-Authored-By: Stanislau <nedbailov375426@gmail.com>
- Track bestTokenLen separately from bestLen (hit span) in extractSnippet so collapsed-token matches don't inflate the comparison threshold - Tokenize raw query instead of pre-normalized lowercase to preserve camelCase splitting (e.g. BoundedContext → bounded + context) - Fix nonsense-query test to use truly unmatchable token Co-Authored-By: Stanislau <nedbailov375426@gmail.com>
Address venikman's two P2 findings: 1. browse() default page now interleaves patterns, routes, and lexemes so all kinds are visible within the default 200-entry cap, instead of burying routes/lexemes past the limit. 2. search() now boosts exact ID matches (+200) and exact title matches (+150) so that searching 'A.1.1' returns A.1.1 first, not A.1. Added tests for both behaviors. Co-Authored-By: Stanislau <nedbailov375426@gmail.com>
c7fc6eb to
7b97ae6
Compare
|
Addressed both P2 findings in 1. Default browse page hides routes and lexemes — 2. Exact-ID search doesn't rank the exact node first —
Searching Added two new tests:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b97ae67eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for (const kind of kinds) { | ||
| const bucket = byKind.get(kind)!; | ||
| result.push(...bucket.splice(0, perKind)); |
There was a problem hiding this comment.
Enforce browse limit when interleaving kinds
In interleaveBrowseEntries, the first pass always appends up to perKind entries from every kind without checking remaining capacity, so requests like browse({ limit: 1 }) (with multiple kinds present) can return more than the requested limit. This breaks the tool’s paging contract and can still overrun caller-side result-size assumptions for small limits. Please cap additions by remaining slots (or truncate before returning) so entries.length <= limit is always true.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already fixed in f1ec6b3 — added result.slice(0, limit) before returning so entries.length <= limit is always enforced, even when limit < kindCount.
When limit is smaller than the number of distinct kinds (e.g. limit=1 with 3 kinds), the first-pass perKind floor of 1 caused more entries than requested. Now truncates to limit before returning. Co-Authored-By: Stanislau <nedbailov375426@gmail.com>
What
Add a public MCP discovery layer with
browse_fpf_catalogandsearch_fpftools, giving agent operators a task-oriented entry path to the spec before drilling into individual nodes.Closes #15
Why
The current MCP surface is strong on answers, tracing, and raw evidence but lacks first-pass discovery for agent operators. Today the default interaction is selector- and debug-oriented. Per issue #15 and FPF grounding (E.14, A.2.3, E.10.D2, A.6.3), agents need an obvious entry path → task-shaped discovery → source opening workflow, with debug/tracing separated from default use.
Type
feat— new capabilityfix— bug fixrefactor— code restructuringdocs— documentation onlychore— maintenance (deps, CI, cleanup)Changes
src/runtime/types.ts— AddCatalogEntry,BrowseCatalogResult,SearchHit,SearchResultinterfaces for the discovery layersrc/runtime/runtime.ts— Addbrowse()andsearch()methods toFpfRuntime:browse(): Lists compiled nodes with optionalpart/status/kindfilters, sorted by IDsearch(): Full-text search using existingscoreOverlap/tokenizeinfrastructure, returns ranked hits with contextual snippetssearch('A.1.1')returnsA.1.1first (notA.1)nodeToCatalogEntry(),extractSnippet(),interleaveBrowseEntries()src/mcp/tool-contracts.ts— Add Zod schemas for browse/search inputs and resultssrc/mcp/tools.ts— AddbrowseFpfCatalogToolandsearchFpfTooldefinitions; reorganize tool families:browse_fpf_catalog,search_fpf,ask_fpf,query_fpf_spec,read_fpf_doc,get_fpf_index_statusinspect_fpf_node,inspect_fpf_anchor,expand_fpf_citations,trace_fpf_pathrefresh_fpf_indexsrc/mastra/mcp/server.ts— Update public surface descriptiontests/mcp-server.test.ts— Update tool list expectationstests/discovery-layer.test.ts— Discovery layer tests + two new tests:'includes all kinds in default unfiltered page'— asserts patterns, routes, and lexemes all present'ranks exact ID match first when searching by node ID'— asserts A.1.1 is top hitValidation
bun run lintpasses locallybun run checkpasses locallybun run testpasses locallybun run buildsucceeds (if runtime/server code touched)bun run docs:buildsucceeds (if docs touched)Boundary check
FPF-spec.mdonly — no additional corpora addedsrc/mcp/tool-contracts.tsAgent metadata
Requested by: @venikman