Skip to content

feat: add public browse/search MCP discovery layer for Codex-facing workflows#33

Merged
venikman merged 6 commits into
mainfrom
devin/1776045631-mcp-discovery-layer
Apr 14, 2026
Merged

feat: add public browse/search MCP discovery layer for Codex-facing workflows#33
venikman merged 6 commits into
mainfrom
devin/1776045631-mcp-discovery-layer

Conversation

@venikman
Copy link
Copy Markdown
Owner

@venikman venikman commented Apr 13, 2026

What

Add a public MCP discovery layer with browse_fpf_catalog and search_fpf tools, 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 capability
  • fix — bug fix
  • refactor — code restructuring
  • docs — documentation only
  • chore — maintenance (deps, CI, cleanup)

Changes

  • src/runtime/types.ts — Add CatalogEntry, BrowseCatalogResult, SearchHit, SearchResult interfaces for the discovery layer
  • src/runtime/runtime.ts — Add browse() and search() methods to FpfRuntime:
    • browse(): Lists compiled nodes with optional part/status/kind filters, sorted by ID
    • [NEW] Default browse page now interleaves kinds (patterns/routes/lexemes) so all kinds are visible within the 200-entry cap, instead of burying routes/lexemes past the limit
    • search(): Full-text search using existing scoreOverlap/tokenize infrastructure, returns ranked hits with contextual snippets
    • [NEW] Exact ID matches get +200 boost, exact title matches get +150, so search('A.1.1') returns A.1.1 first (not A.1)
    • Helper functions: nodeToCatalogEntry(), extractSnippet(), interleaveBrowseEntries()
  • src/mcp/tool-contracts.ts — Add Zod schemas for browse/search inputs and results
  • src/mcp/tools.ts — Add browseFpfCatalogTool and searchFpfTool definitions; reorganize tool families:
    • Public: browse_fpf_catalog, search_fpf, ask_fpf, query_fpf_spec, read_fpf_doc, get_fpf_index_status
    • Expert: inspect_fpf_node, inspect_fpf_anchor, expand_fpf_citations, trace_fpf_path
    • Admin: refresh_fpf_index
  • src/mastra/mcp/server.ts — Update public surface description
  • tests/mcp-server.test.ts — Update tool list expectations
  • tests/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 hit

Validation

  • bun run lint passes locally
  • bun run check passes locally
  • bun run test passes locally
  • No new warnings introduced
  • bun run build succeeds (if runtime/server code touched)
  • bun run docs:build succeeds (if docs touched)
  • Relevant docs updated (README, docs/, inline JSDoc if applicable)

Boundary check

  • Runtime source set is FPF-spec.md only — no additional corpora added
  • No vector database or remote indexing introduced
  • No Python code added
  • MCP tool contracts stay in src/mcp/tool-contracts.ts

Agent metadata

Field Value
Agent Devin
Session https://app.devin.ai/sessions/8fa4d238c77d45b2bfa931f5547007bc
Prompt Merger: address venikman's P2 findings on #33 (browse kind interleaving + exact-ID search boost)

Requested by: @venikman

Copilot AI review requested due to automatic review settings April 13, 2026 02:10
@devin-ai-integration
Copy link
Copy Markdown
Contributor

Original prompt from Stanislau

how our issues doing?

@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 13, 2026

Warning

Rate limit exceeded

@devin-ai-integration[bot] has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 43 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e2484777-c1b6-4e84-840f-548c3ef3abbb

📥 Commits

Reviewing files that changed from the base of the PR and between 448c95f and 124daae.

📒 Files selected for processing (8)
  • AGENTS.md
  • src/mastra/mcp/server.ts
  • src/mcp/tool-contracts.ts
  • src/mcp/tools.ts
  • src/runtime/runtime.ts
  • src/runtime/types.ts
  • tests/discovery-layer.test.ts
  • tests/mcp-server.test.ts
📝 Walkthrough

Walkthrough

Adds 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

Cohort / File(s) Summary
MCP Server Configuration
src/mastra/mcp/server.ts
Updated fpfMemoryPublic description to advertise a "public discovery surface (browse, search, ask, query, read)".
MCP Tool Contracts
src/mcp/tool-contracts.ts
Added Zod schemas and exported types for discovery: catalogEntrySchema, browseFpfCatalogInputSchema, browseFpfCatalogResultSchema, searchHitSchema, searchFpfInputSchema, searchFpfResultSchema, plus inferred BrowseFpfCatalogInput and SearchFpfInput.
MCP Tool Definitions
src/mcp/tools.ts
Added browse_fpf_catalog and search_fpf tools, created fpfAdminTools (index ops), moved/admin-separated index tools, adjusted fpfPublicTools and fpfExpertTools, and aggregated into fpfMcpTools.
Runtime Implementation
src/runtime/runtime.ts
Implemented FpfRuntime.browse() and FpfRuntime.search() with snapshot refresh, filtering, scoring, snippet extraction, and helpers (nodeToCatalogEntry, snippet utilities).
Runtime Type Definitions
src/runtime/types.ts
Added discovery-layer interfaces: CatalogEntry, BrowseCatalogResult, SearchHit, and SearchResult (including snapshot metadata).
Tests
tests/mcp-server.test.ts, tests/discovery-layer.test.ts
Updated MCP server tool-list expectations; added comprehensive discovery-layer tests exercising browse/search behavior, filtering, limits, ordering, snippets, and snapshot metadata.
Docs
AGENTS.md
Documented new public tools (browse_fpf_catalog, search_fpf), moved index ops to an Admin tools section, and reclassified read_fpf_doc into Public tools.

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
Loading
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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I hop the catalog, nibbling at bits of lore,

Browsing, searching, finding paths to explore.
Snippets and scores light the rabbit trail,
Public discovery blows a windy trail of tail.
Hooray — agents find routes, and I find more! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely describes the main change: adding a public browse/search MCP discovery layer for Codex-facing workflows, which aligns with the primary objective of the changeset.
Description check ✅ Passed The PR description comprehensively covers all required template sections including What, Why, Type, detailed Changes, Validation results, and Boundary checks. Agent metadata is also provided.
Linked Issues check ✅ Passed The code changes fully address issue #15 objectives: new browse_fpf_catalog and search_fpf tools added, tools reorganized into Public/Expert/Admin families, runtime methods implemented, Zod schemas defined, and tests updated accordingly.
Out of Scope Changes check ✅ Passed All changes are scoped to the linked issue #15 objectives: discovery layer implementation, tool organization, and documentation updates. No unrelated changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch devin/1776045631-mcp-discovery-layer

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/mcp/tool-contracts.ts
Comment on lines +475 to +482
export const browseFpfCatalogInputSchema = z
.object({
part: z.string().optional(),
status: z.string().optional(),
kind: nodeKindSchema.optional(),
forceRefresh: z.boolean().optional(),
})
.strict();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/runtime/runtime.ts Outdated
Comment on lines +228 to +242
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);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/runtime/runtime.ts Outdated
Comment on lines +452 to +454
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();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The snippet extraction logic currently slices the text at fixed offsets, which can result in words being cut in half at the beginning or end of the snippet. Consider adjusting the boundaries to the nearest whitespace to provide cleaner, more readable snippets.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed — snippet boundaries are now snapped to the nearest whitespace so words aren't cut in half at either end.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_catalog and search_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.

Comment thread src/runtime/runtime.ts Outdated
Comment on lines +439 to +448
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;
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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;

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/runtime/runtime.ts
Comment on lines +222 to +256
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 },
};
}
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 13 focused discovery-layer tests in tests/discovery-layer.test.ts covering:

  • browse(): unfiltered results, sort order, kind/part/status filters, limit, descriptions
  • search(): ranked hits, score ordering, snippets, kind filter, limit, zero-hit case, dotted-ID snippet centering

All passing alongside the existing 39 tests.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/runtime/runtime.ts Outdated
Comment on lines +228 to +230
let entries: CatalogEntry[] = Object.values(snapshot.compiledNodes).map((node) =>
nodeToCatalogEntry(node, snapshot),
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/runtime/runtime.ts Outdated
Comment on lines +263 to +264
const normalizedQuery = normalizeForLookup(query);
const queryTokens = tokenize(normalizedQuery);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@devin-ai-integration devin-ai-integration Bot force-pushed the devin/1776045631-mcp-discovery-layer branch 2 times, most recently from 18dd0dd to 124daae Compare April 13, 2026 17:38
@venikman
Copy link
Copy Markdown
Owner Author

Code Review

Overview

Adds two new public MCP tools — browse_fpf_catalog (filtered listing by part/status/kind) and search_fpf (full-text ranked search with snippets) — plus reshuffles the public/expert/admin tool buckets.

Strengths

  • Directly improves the indexing goal — browse and search are exactly the discovery primitives Codex-facing workflows need before drilling into ask/query. This is high-value surface area.
  • Snippet logic is thoughtful — the dotted-ID regex (A.1.1 → pattern allowing [^a-z0-9]* between chars) is a nice touch for FPF-style identifiers that tokenizers typically fragment.
  • Word-boundary snapping in extractSnippet avoids mid-word truncation.
  • Good test coveragetests/discovery-layer.test.ts covers filtering, sorting, limits, snippet centering, and kind-filtered search.
  • Bounded limitsmin(500) and min(100) caps keep adversarial clients honest.

Issues & Risks

🔴 Conflicts with PR #36 hosted surface

PR #36 (already open) states the hosted public surface is ask_fpf, query_fpf_spec, get_fpf_index_status. This PR changes the public bucket to browse_fpf_catalog, search_fpf, ask_fpf, query_fpf_spec, read_fpf_doc — and moves get_fpf_index_status out to a new admin bucket.

Two concrete problems:

  1. manifest.json and server.json advertise the old public surface — this PR changes the code but I didn't see those updated.
  2. If Use hosted MCP by default and harden deploy runtime paths #36 merges first, the hosted MCP will expose the old 3 tools. When feat: add public browse/search MCP discovery layer for Codex-facing workflows #33 merges, the hosted bundle suddenly adds 2 new tools and drops get_fpf_index_status. Clients relying on get_fpf_index_status from the hosted endpoint will break.

Please either:

browse() and search() unconditionally call this.refresh(false)

await this.refresh(options.forceRefresh ?? false);

Even refresh(false) takes the fast path most of the time, but still runs hashFile(this.sourcePath) and touches the filesystem on every browse/search call. For a search-driven UI (many calls per keystroke) this adds up. Consider:

  • Cache the source-hash check with a short TTL (a few seconds), or
  • Skip the refresh call when forceRefresh is false and the snapshot is already loaded

extractSnippet constructs a regex per token per node

const pattern = new RegExp(escaped.join('[^a-z0-9]*'), 'i');

For a query like "A.1.1" that collapses to a 3-char token, this runs once per node (fine). For multi-token queries, it's per token per node — still fine for 200 nodes but worth noting. A precompiled regex per unique token, hoisted above the node loop, avoids the repeated construction.

Scoring is token-overlap only

scoreOverlap(queryTokens, node.searchableText) — no IDF weighting. Common tokens ("pattern", "route") will dominate. For the current corpus this may be acceptable, but a naive BM25 or TF-IDF would improve ranking materially with modest code.

snippet test is weak

if (hit) {
  expect(hit.snippet.toLowerCase()).toMatch(/a\.1\.1|boundedcontext|semantic/i);
}

The if (hit) makes this a no-op when the hit is absent — false positives possible. Assert the hit exists first: expect(hit).toBeDefined().

Nits

  • NodeKind as NodeKind | undefined casts in tools.ts are unnecessary — the zod schema already narrows the type. Remove the as casts.
  • AGENTS.md tool categorization now diverges from manifest.json; reconcile.

Recommendation

Hold pending coordination with #36 on the hosted public surface. The browse/search feature itself is a clear win and well-tested; the surface-change collision is the blocking issue.

devin-ai-integration Bot added a commit that referenced this pull request Apr 14, 2026
PR #36 advertises get_fpf_index_status as public in the hosted MCP.
Moving it to admin in #33 would break callers. Keep it public and
move only refresh_fpf_index to admin.

Co-Authored-By: Stanislau <nedbailov375426@gmail.com>
@devin-ai-integration devin-ai-integration Bot force-pushed the devin/1776045631-mcp-discovery-layer branch from 124daae to c7fc6eb Compare April 14, 2026 04:27
@devin-ai-integration
Copy link
Copy Markdown
Contributor

Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it.

Copy link
Copy Markdown
Owner Author

@venikman venikman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two review findings:

  1. 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.

  1. 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.

devin-ai-integration Bot and others added 5 commits April 14, 2026 06:02
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>
PR #36 advertises get_fpf_index_status as public in the hosted MCP.
Moving it to admin in #33 would break callers. Keep it public and
move only refresh_fpf_index to admin.

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>
@devin-ai-integration devin-ai-integration Bot force-pushed the devin/1776045631-mcp-discovery-layer branch from c7fc6eb to 7b97ae6 Compare April 14, 2026 06:03
@devin-ai-integration
Copy link
Copy Markdown
Contributor

Addressed both P2 findings in 7b97ae6:

1. Default browse page hides routes and lexemesbrowse() now interleaves entries by kind when no kind filter is active. Each kind gets floor(limit / kindCount) slots, with remaining slots filled round-robin. On the current snapshot the default 200-entry page now includes patterns, routes, and lexemes. When a kind filter is explicitly set, the old behavior (simple sort + slice) is preserved.

2. Exact-ID search doesn't rank the exact node firstsearch() now applies an explicit ranking boost before the generic overlap sort:

  • Exact ID match (case-insensitive via normalizeForLookup): +200
  • Exact title match: +150

Searching A.1.1 now returns A.1.1 first, ahead of A.1.

Added two new tests:

  • 'includes all kinds in default unfiltered page' — asserts patterns, routes, and lexemes are all present
  • 'ranks exact ID match first when searching by node ID' — asserts A.1.1 is the top hit

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/runtime/runtime.ts
Comment on lines +501 to +503
for (const kind of kinds) {
const bucket = byKind.get(kind)!;
result.push(...bucket.splice(0, perKind));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already fixed in f1ec6b3 — added result.slice(0, limit) before returning so entries.length <= limit is always enforced, even when limit < kindCount.

devin-ai-integration[bot]

This comment was marked as resolved.

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>
@venikman venikman merged commit ce6eb38 into main Apr 14, 2026
1 check passed
@venikman venikman deleted the devin/1776045631-mcp-discovery-layer branch May 9, 2026 10:59
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.

Add public browse/search MCP discovery layer for Codex-facing workflows

2 participants