feat(mcp-server, query-core, dialtone-docs, dialtone-cli): DLT-3416 add search_documentation tool#1259
Open
belumontoya wants to merge 2 commits into
Open
feat(mcp-server, query-core, dialtone-docs, dialtone-cli): DLT-3416 add search_documentation tool#1259belumontoya wants to merge 2 commits into
belumontoya wants to merge 2 commits into
Conversation
…dd search_documentation tool Adds a 5th MCP tool that searches the public docs site corpus (apps/dialtone-documentation/docs/) for prose-y questions: usage guidance, recipes, accessibility rules, migration guides, design principles. Fills the gap left by the 4 existing tools, which only return mechanical names/values. - New producer in dialtone-docs chunks ~196 markdown files into ~1297 heading-bounded sections (status blacklist filter, VuePress directive stripping via existing stripMarkdown utility) - searchDocumentation in dialtone-query-core uses hand-rolled regex word matching with stop-word filtering and tier scoring (heading match ranks above content-only match) - search_documentation tool + dialtone://documentation resource registered in dialtone-mcp-server transport - dialtone docs <query> CLI subcommand + unified search aggregation - 10/10 acceptance scenarios pass (bar: >= 8) via the new test:acceptance harness in dialtone-mcp-server/scripts/ 85 new tests: 60 producer (chunking, frontmatter, status filter, edge cases) + 15 search unit + 10 acceptance scenarios. Part of DLT-3404 epic. Blocks DLT-3110 (internal corpus sibling).
Contributor
|
Adds Overall Judgement: ✅ Ready to merge All acceptance tests pass (10/10), feature is fully integrated across MCP server, CLI, and query core with comprehensive documentation coverage and performance targets met. WalkthroughAdds a documentation search feature to Dialtone, converting component markdown files into a searchable JSON corpus via a new build pipeline. Implements AND-logic search with tiered scoring, adds CLI ChangesDocumentation Search
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dialtone-mcp-server/src/index.ts (1)
270-276: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winSchema default vs runtime default mismatch.
Line 276 sets
limit = args.limit || 15, but thesearch_documentationtool schema declaresdefault: 10(line 257). This means clients see "default 10" in the schema, but if they omitlimit, the server uses 15.This pre-existing issue also affects
search_components(schema: 10, runtime: 15) andsearch_icons(schema: 20, runtime: 15).Consider using the schema default or making the runtime fallback match each tool's schema.
💡 Suggested fix to honor per-tool defaults
const toolName = request.params.name; const args = request.params.arguments || {}; const query = args.query; - const limit = args.limit || 15; + + // Use per-tool defaults from schema + const defaultLimits: Record<string, number> = { + search_utility_classes: 15, + search_tokens: 15, + search_components: 10, + search_icons: 20, + search_documentation: 10, + }; + const limit = args.limit ?? defaultLimits[toolName] ?? 15;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dialtone-mcp-server/src/index.ts` around lines 270 - 276, The server currently sets limit = args.limit || 15 inside server.server.setRequestHandler (CallToolRequestSchema) which mismatches schema defaults for search_documentation, search_components and search_icons; update the handler to honor per-tool defaults by determining the default based on request.params.name (e.g. use a small mapping like TOOL_DEFAULT_LIMITS for "search_documentation" => 10, "search_components" => 10, "search_icons" => 20) and set limit = args.limit ?? TOOL_DEFAULT_LIMITS[toolName] ?? 15 (or equivalent nullish-coalescing logic) so runtime fallbacks match the declared tool schema defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/dialtone-documentation/docs/components/combobox.md`:
- Line 13: The sentence beginning "A combobox provides accessibility controls
and common functionality for search inputs with autocomplete and filtering."
contains a grammatical error: replace the contraction "it's own" with the
possessive "its own" (so the clause reads "It does not render any functioning UI
on its own"). Update the text in the combobox documentation to use "its own".
In `@packages/dialtone-cli/src/commands/docs.ts`:
- Line 14: The code currently assigns const limit = Number(args.limit) which
allows negatives, decimals and NaN causing incorrect slice(0, -n) behavior and
wrong "more" counts; change the parsing of args.limit to a validated
non-negative integer (e.g., parseInt or Number but then check Number.isInteger
and >= 0), fail fast with an error when invalid, and clamp the value to the
results length before using slice; update usages where limit is used (the local
variable limit and the slices at slice(0, limit) and any calculation of "more")
so they operate on the validated/clamped integer.
- Around line 7-11: Replace the hardcoded English strings in this command's
public API with FTL localization keys: change meta.description and each args.*
description/default (meta.description, args.query.description,
args.format.description, args.format.default, args.limit.description,
args.limit.default) to use FTL lookup calls referencing keys like
"docs.meta.description", "docs.args.query.description",
"docs.args.format.description", "docs.args.format.default", and
"docs.args.limit.description" (and add those keys to the project's .ftl resource
file); ensure the code uses the project's i18n/FTL accessor function when
assigning these values so all user-facing text is pulled from the localization
catalog.
In `@packages/dialtone-docs/package.json`:
- Around line 17-19: The package.json currently exposes all files via the
exports wildcard ("./*": "./*"); replace this broad export with an explicit
subpath export for only the file consumers should import (e.g., export
"./dist/public-docs.json": "./dist/public-docs.json") so that only
dist/public-docs.json is importable and internal files remain hidden; update the
"exports" object in packages/dialtone-docs/package.json accordingly.
In `@packages/dialtone-docs/src/generators/build-public-docs.mjs`:
- Line 124: frontmatter.status is checked case-sensitively against
NON_READY_STATUSES so values like "WIP" or "Beta" slip through; normalize the
status before the blacklist check by converting frontmatter.status to a
canonical case (e.g. lower-case) and test that normalized value against
NON_READY_STATUSES (or ensure NON_READY_STATUSES contains lower-case entries),
e.g. compute a normalizedStatus from String(frontmatter.status) and use
NON_READY_STATUSES.has(normalizedStatus) in the condition that returns [].
- Line 70: The current fence detection only matches fences at column 0; update
the regex used in the assignment to fenceMatch (the line.match call) to allow up
to three leading spaces so indented fenced code blocks are recognized (e.g.
change /^(`{3,}|~{3,})/ to a pattern that permits 0–3 leading spaces like /^[
]{0,3}(`{3,}|~{3,})/), keeping the rest of the fence logic intact so headings
inside indented code blocks aren’t misclassified as sections.
In `@packages/dialtone-mcp-server/scripts/run-acceptance-scenarios.mjs`:
- Line 16: The hardcoded PASS_BAR constant (PASS_BAR = 8) assumes exactly 10
scenarios and can be impossible to meet if acceptance-scenarios.json changes;
update the script to compute PASS_BAR dynamically (e.g., PASS_BAR =
Math.ceil(scenarios.length * 0.8)) or, if you want to keep a fixed threshold,
validate the input by checking scenarios.length === 10 and throw/log a clear
error; adjust any references to PASS_BAR and the scenario loading logic so
PASS_BAR is derived after reading scenarios.length (use the variable name
scenarios and the constant PASS_BAR where present).
In `@packages/dialtone-query-core/src/tools/docs.ts`:
- Around line 51-63: The query normalization allows arbitrarily long input and
unbounded token counts, which can cause expensive scans; modify the logic around
normalized/allWords/words to enforce a MAX_QUERY_LENGTH (e.g., truncate the raw
query string before normalization) and a MAX_TERM_COUNT (e.g., slice
allWords/meaningful to that limit) and return an early note when truncation
occurs; update the code paths that use normalized, allWords, meaningful, and
words and reference STOP_WORDS so the token-limit is applied after stop-word
filtering but also enforce a hard limit on allWords to avoid worst-case
behavior.
In `@packages/dialtone-query-core/src/types.ts`:
- Around line 81-87: The DocumentationFrontmatter interface's status property
currently uses "status?: 'ready' | 'planned' | 'beta' | 'wip' | string", which
weakens type safety; update the status declaration on the
DocumentationFrontmatter interface to a strict union of allowed literal values
(remove the trailing "| string") if the only valid statuses are "ready" |
"planned" | "beta" | "wip", or alternatively define and reference a named
type/enum (e.g., DocumentationStatus) that lists the permitted literals so the
DocumentationFrontmatter.status uses that strict type and catches typos at
compile time.
---
Outside diff comments:
In `@packages/dialtone-mcp-server/src/index.ts`:
- Around line 270-276: The server currently sets limit = args.limit || 15 inside
server.server.setRequestHandler (CallToolRequestSchema) which mismatches schema
defaults for search_documentation, search_components and search_icons; update
the handler to honor per-tool defaults by determining the default based on
request.params.name (e.g. use a small mapping like TOOL_DEFAULT_LIMITS for
"search_documentation" => 10, "search_components" => 10, "search_icons" => 20)
and set limit = args.limit ?? TOOL_DEFAULT_LIMITS[toolName] ?? 15 (or equivalent
nullish-coalescing logic) so runtime fallbacks match the declared tool schema
defaults.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 74e47fe0-ac0c-4893-8d98-3b84603f39ce
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by**
📒 Files selected for processing (40)
.claude/rules/mcp-server.mdCLAUDE.mdapps/dialtone-documentation/docs/components/button.mdapps/dialtone-documentation/docs/components/combobox-multi-select.mdapps/dialtone-documentation/docs/components/combobox.mdapps/dialtone-documentation/docs/components/input.mdapps/dialtone-documentation/docs/components/modal.mdapps/dialtone-documentation/docs/components/popover.mdapps/dialtone-documentation/docs/components/select-menu.mdapps/dialtone-documentation/docs/components/tooltip.mdapps/dialtone-documentation/docs/guides/mcp-server/index.mdpackages/dialtone-cli/src/commands/docs.tspackages/dialtone-cli/src/commands/search.tspackages/dialtone-cli/src/context.tspackages/dialtone-cli/src/data-resolver.tspackages/dialtone-cli/src/index.tspackages/dialtone-docs/package.jsonpackages/dialtone-docs/project.jsonpackages/dialtone-docs/src/generators/build-public-docs.mjspackages/dialtone-docs/tests/fixtures/public-docs/beta.mdpackages/dialtone-docs/tests/fixtures/public-docs/complete.mdpackages/dialtone-docs/tests/fixtures/public-docs/multiline-directive.mdpackages/dialtone-docs/tests/fixtures/public-docs/nested-headings.mdpackages/dialtone-docs/tests/fixtures/public-docs/no-headings.mdpackages/dialtone-docs/tests/fixtures/public-docs/no-status.mdpackages/dialtone-docs/tests/fixtures/public-docs/planned.mdpackages/dialtone-docs/tests/fixtures/public-docs/unknown-status.mdpackages/dialtone-docs/tests/tests/build-public-docs.test.jspackages/dialtone-mcp-server/package.jsonpackages/dialtone-mcp-server/scripts/acceptance-scenarios.jsonpackages/dialtone-mcp-server/scripts/run-acceptance-scenarios.mjspackages/dialtone-mcp-server/src/index.tspackages/dialtone-query-core/package.jsonpackages/dialtone-query-core/project.jsonpackages/dialtone-query-core/src/data.tspackages/dialtone-query-core/src/index.tspackages/dialtone-query-core/src/tools/docs.tspackages/dialtone-query-core/src/types.tspackages/dialtone-query-core/tests/tools/docs.test.tspackages/dialtone-query-core/vitest.config.ts
…dress PR #1259 review - combobox.md: fix 'it's own' grammar - dialtone-cli/docs.ts: validate --limit (parseInt + non-negative integer guard) - dialtone-docs/package.json: narrow exports to dist/public-docs.json only - build-public-docs.mjs: allow up-to-3-space-indented fenced code blocks - build-public-docs.mjs: case-insensitive status check (catches WIP/Beta typos) - run-acceptance-scenarios.mjs: guard against scenario count drift - query-core/docs.ts: bound query length (256 chars) and term count (12) - query-core/types.ts: drop '| string' fallback from status union
Contributor
|
✔️ Deploy previews ready! |
belumontoya
requested review from
Brad Paugh (braddialpad),
Francis Rupert (francisrupert),
Ignacio Ropolo (iropolo) and
Nina Repetto (ninarepetto)
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f39c2c4097
ℹ️ 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 (@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 (@codex) address that feedback".
| ? basename(dirname(absolutePath)) | ||
| : (relParts.length > 1 ? relParts[0] : 'root'); | ||
|
|
||
| const docId = name; |
There was a problem hiding this comment.
buildRecords assigns docId from basename(absolutePath), which creates collisions for many public docs that share filenames (for example multiple index.md files under different directories). This makes docId and derived record IDs ambiguous across unrelated documents (index#...), so downstream consumers cannot reliably identify or deduplicate a specific doc section by ID.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/dialtone-cli/src/commands/docs.ts (1)
14-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject partial parses for
--limit.
Number.parseIntstill accepts values like1.5,10abc, and0x10, so this path can silently run with the wrong limit instead of failing fast.Suggested fix
- const limit = Number.parseInt(args.limit, 10); - if (!Number.isInteger(limit) || limit < 0) { + if (!/^(0|[1-9]\d*)$/.test(args.limit)) { console.error(`Invalid --limit value "${args.limit}". Expected a non-negative integer (0 = no limit).`); process.exit(1); } + const limit = Number(args.limit);Run this to verify the current behavior. Expected:
1.5,10abc, and0x10should currently showaccepted=true, confirming the guard is too permissive.#!/bin/bash set -euo pipefail sed -n '13,18p' packages/dialtone-cli/src/commands/docs.ts node <<'NODE' for (const value of ['1.5', '10abc', '0x10', '0', '10', '-1', 'abc']) { const limit = Number.parseInt(value, 10); console.log(`${value} -> ${limit} | accepted=${Number.isInteger(limit) && limit >= 0}`); } NODE🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dialtone-cli/src/commands/docs.ts` around lines 14 - 18, The current parse of args.limit uses Number.parseInt which accepts partial/non-decimal inputs (e.g. "1.5", "10abc", "0x10"); update the validation around the limit variable so it only accepts a non-negative integer string. Specifically, replace the Number.parseInt-based check for limit (the const limit = Number.parseInt(args.limit, 10) and its subsequent guard) with a strict string check (e.g. /^\d+$/) against args.limit, then convert to a Number (or parseInt) only after it matches and enforce Number.isInteger(limit) && limit >= 0; keep references to args.limit and the limit variable so tests and error message remain the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/dialtone-query-core/src/tools/docs.ts`:
- Around line 16-31: The STOP_WORDS list contains a duplicate entry for the
token 'so'; remove the redundant 'so' from the array in
packages/dialtone-query-core/src/tools/docs.ts so the STOP_WORDS constant
contains each stop word only once (keep a single 'so' entry and delete the
other).
In `@packages/dialtone-query-core/tests/tools/docs.test.ts`:
- Around line 193-270: The test hardcodes an "8 of 10" acceptance bar but never
asserts scenarios still contains 10 items, so add a guard in the same test (or a
separate tiny test) to ensure scenarios.length is as expected or compute the
threshold dynamically; locate the scenarios array and the test named '≥ 8 of 10
scenarios return a relevant top-3 result' and either assert
expect(scenarios.length).toBe(10) before computing `passed` or replace the fixed
8 with a derived value like Math.ceil(scenarios.length * 0.8) (use the existing
`passed` and `results` variables) so adding/removing cases won't silently change
the acceptance criteria.
---
Duplicate comments:
In `@packages/dialtone-cli/src/commands/docs.ts`:
- Around line 14-18: The current parse of args.limit uses Number.parseInt which
accepts partial/non-decimal inputs (e.g. "1.5", "10abc", "0x10"); update the
validation around the limit variable so it only accepts a non-negative integer
string. Specifically, replace the Number.parseInt-based check for limit (the
const limit = Number.parseInt(args.limit, 10) and its subsequent guard) with a
strict string check (e.g. /^\d+$/) against args.limit, then convert to a Number
(or parseInt) only after it matches and enforce Number.isInteger(limit) && limit
>= 0; keep references to args.limit and the limit variable so tests and error
message remain the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b8f31aea-d68c-4732-a2a6-97eb208e059f
📒 Files selected for processing (11)
apps/dialtone-documentation/docs/components/combobox.mdpackages/dialtone-cli/src/commands/docs.tspackages/dialtone-docs/package.jsonpackages/dialtone-docs/src/generators/build-public-docs.mjspackages/dialtone-docs/tests/fixtures/public-docs/indented-fence.mdpackages/dialtone-docs/tests/fixtures/public-docs/uppercase-status.mdpackages/dialtone-docs/tests/tests/build-public-docs.test.jspackages/dialtone-mcp-server/scripts/run-acceptance-scenarios.mjspackages/dialtone-query-core/src/tools/docs.tspackages/dialtone-query-core/src/types.tspackages/dialtone-query-core/tests/tools/docs.test.ts
Comment on lines
+16
to
+31
| 'a', 'an', 'the', 'and', 'or', 'but', 'nor', 'for', 'yet', 'so', | ||
| 'in', 'on', 'at', 'by', 'to', 'of', 'up', 'as', 'if', 'is', | ||
| 'into', 'onto', 'from', 'with', 'about', 'above', 'below', 'between', | ||
| 'through', 'during', 'before', 'after', 'against', 'among', 'around', | ||
| // Auxiliaries | ||
| 'be', 'been', 'being', 'are', 'was', 'were', 'have', 'has', 'had', | ||
| 'do', 'does', 'did', 'will', 'would', 'should', 'could', 'may', | ||
| 'might', 'shall', 'can', 'cannot', | ||
| // Pronouns | ||
| 'i', 'me', 'my', 'we', 'our', 'you', 'your', 'he', 'she', 'it', | ||
| 'its', 'they', 'their', 'this', 'that', 'these', 'those', | ||
| // Question words | ||
| 'what', 'which', 'who', 'whom', 'whose', 'how', 'when', 'where', 'why', | ||
| // Quantifiers / adverbs | ||
| 'all', 'both', 'each', 'few', 'more', 'most', 'other', 'some', 'such', | ||
| 'no', 'not', 'so', 'than', 'too', 'very', 'just', 'also', 'now', 'then', |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Duplicate entry in STOP_WORDS.
'so' appears on both line 16 and line 31. Harmless (Set dedupes), but removes cleanly.
🧹 Remove duplicate
// Quantifiers / adverbs
- 'all', 'both', 'each', 'few', 'more', 'most', 'other', 'some', 'such',
- 'no', 'not', 'so', 'than', 'too', 'very', 'just', 'also', 'now', 'then',
+ 'all', 'both', 'each', 'few', 'more', 'most', 'other', 'some', 'such',
+ 'no', 'not', 'than', 'too', 'very', 'just', 'also', 'now', 'then',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dialtone-query-core/src/tools/docs.ts` around lines 16 - 31, The
STOP_WORDS list contains a duplicate entry for the token 'so'; remove the
redundant 'so' from the array in packages/dialtone-query-core/src/tools/docs.ts
so the STOP_WORDS constant contains each stop word only once (keep a single 'so'
entry and delete the other).
Comment on lines
+193
to
+270
| const scenarios: Scenario[] = [ | ||
| { | ||
| id: 'TS-002', | ||
| query: "What component do I use for a search box with autocomplete?", | ||
| allowlist: ['combobox', 'search-input'], | ||
| }, | ||
| { | ||
| id: 'TS-003', | ||
| query: "What's the difference between DtButton kind='primary' and kind='danger'?", | ||
| allowlist: ['button'], | ||
| }, | ||
| { | ||
| id: 'TS-004', | ||
| query: "Why isn't my DtModal closing on outside click — is that correct behavior?", | ||
| allowlist: ['modal'], | ||
| }, | ||
| { | ||
| id: 'TS-005', | ||
| query: "DtOldPopover is deprecated — what's the replacement?", | ||
| allowlist: ['popover'], | ||
| }, | ||
| { | ||
| id: 'TS-006', | ||
| query: "Which Dialtone component supports multi-select with avatars?", | ||
| allowlist: ['combobox-multi-select', 'combobox'], | ||
| }, | ||
| { | ||
| id: 'TS-007', | ||
| query: "How do I wire DtSelectMenu v-model to a Vuex store?", | ||
| allowlist: ['select-menu'], | ||
| }, | ||
| { | ||
| id: 'TS-008', | ||
| query: "Can I put DtTooltip on a disabled DtButton?", | ||
| allowlist: ['tooltip', 'button'], | ||
| }, | ||
| { | ||
| id: 'TS-009', | ||
| query: "List all components that support dark mode.", | ||
| allowlist: [], // any result is acceptable — cross-cutting query | ||
| }, | ||
| { | ||
| id: 'TS-010', | ||
| query: "How do I make DtButton show a loading spinner during async submit?", | ||
| allowlist: ['button'], | ||
| }, | ||
| { | ||
| id: 'TS-011', | ||
| query: "Why does DtInput show a red border?", | ||
| allowlist: ['input'], | ||
| }, | ||
| ]; | ||
|
|
||
| test('corpus is loaded and non-empty', () => { | ||
| expect(documentation.length).toBeGreaterThan(1000); | ||
| }); | ||
|
|
||
| // Run all 10 scenarios and assert ≥ 8 pass (acceptance bar per PRD) | ||
| test('≥ 8 of 10 scenarios return a relevant top-3 result', () => { | ||
| const results = scenarios.map(({ id, query, allowlist }) => { | ||
| const { results: hits } = searchDocumentation(query, documentation); | ||
| const top3DocIds = hits.slice(0, 3).map(r => (r.details as DocumentationRecord).docId); | ||
| const pass = allowlist.length > 0 | ||
| ? allowlist.some(d => top3DocIds.includes(d)) | ||
| : top3DocIds.length > 0; | ||
| return { id, pass, top3DocIds, query: query.slice(0, 40) }; | ||
| }); | ||
|
|
||
| const passed = results.filter(r => r.pass).length; | ||
| const failing = results.filter(r => !r.pass); | ||
|
|
||
| if (failing.length > 0) { | ||
| console.warn('Failing scenarios (corpus vocabulary gaps to fix):'); | ||
| failing.forEach(f => console.warn(` ${f.id}: "${f.query}..." — got top3: ${f.top3DocIds}`)); | ||
| } | ||
|
|
||
| expect(passed).toBeGreaterThanOrEqual(8); | ||
| }); |
Contributor
There was a problem hiding this comment.
Guard the fixed 8-of-10 acceptance bar.
This test hardcodes an 8-pass threshold, but nothing asserts that scenarios still has 10 entries. Adding or removing a case silently weakens or tightens the harness.
Suggested fix
const scenarios: Scenario[] = [
{
id: 'TS-002',
query: "What component do I use for a search box with autocomplete?",
allowlist: ['combobox', 'search-input'],
@@
},
];
+ test('scenario list stays at 10 cases', () => {
+ expect(scenarios).toHaveLength(10);
+ });
+
test('corpus is loaded and non-empty', () => {
expect(documentation.length).toBeGreaterThan(1000);
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dialtone-query-core/tests/tools/docs.test.ts` around lines 193 -
270, The test hardcodes an "8 of 10" acceptance bar but never asserts scenarios
still contains 10 items, so add a guard in the same test (or a separate tiny
test) to ensure scenarios.length is as expected or compute the threshold
dynamically; locate the scenarios array and the test named '≥ 8 of 10 scenarios
return a relevant top-3 result' and either assert
expect(scenarios.length).toBe(10) before computing `passed` or replace the fixed
8 with a derived value like Math.ceil(scenarios.length * 0.8) (use the existing
`passed` and `results` variables) so adding/removing cases won't silently change
the acceptance criteria.
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.
feat(mcp-server, query-core, dialtone-docs, dialtone-cli): DLT-3416 add search_documentation tool
Obligatory GIF (super important!)
🛠️ Type Of Change
These types will increment the version number on release:
📖 Jira Ticket
https://dialpad.atlassian.net/browse/DLT-3416
📖 Description
Adds a 5th MCP tool
search_documentationand matchingdialtone://documentationresource. The tool searches the public docs site corpus (apps/dialtone-documentation/docs/) — usage prose, recipes, accessibility rules, migration guides, design principles. Surfaced indialtone-clias thedialtone docs <query>subcommand and aggregated into the unifieddialtone search.What ships:
packages/dialtone-docs/src/generators/build-public-docs.mjs): chunks ~196 markdown files into ~1297 heading-bounded sections (H2/H3). Reuses the existingstripMarkdownutility to strip VuePress directives. Skips docs withstatusin{planned, beta, wip}; includes everything else (covers guides, design, whats-new, about pages — 132 of 196 docs have nostatusfield).packages/dialtone-query-core/src/tools/docs.ts): hand-rolled regex word matching, matching the existing 4 tools' pattern. Adds stop-word filtering and tier scoring (heading match ranks above content-only).packages/dialtone-mcp-server/src/index.ts): registers the tool, the dispatcher branch, and thedialtone://documentationresource. Bumps tool count 4 → 5, resource count 5 → 6.packages/dialtone-cli/src/commands/docs.ts): citty subcommand parallel tocomponent/token/utility. Documentation results also flow through unifieddialtone searchaggregation.packages/dialtone-mcp-server/scripts/run-acceptance-scenarios.mjs): runs all 10 scenarios from the PRD against the search engine. Pass bar: ≥ 8/10. Currently 10/10.dialtone-docs:build→dialtone-query-core:build→dialtone-mcp-server:build. Touching any markdown underapps/dialtone-documentation/docs/**invalidates the public-docs.json cache.What changed in
apps/dialtone-documentation/docs/: 9 targeted prose additions to bridge vocabulary gaps that the 10 acceptance scenarios needed (DtButton kind semantics in Variants, DtModal outside-click behavior in Usage, DtTooltip on disabled DtButton in Tooltip-as-Component, DtOldPopover deprecation note in Popover Usage, DtSelectMenu v-model wiring in Select Menu Usage, multi-select-with-avatars in Combobox Multi-Select Usage, autocomplete in Combobox Base Style, DtInput red-border validation state in Input With Validation States, DtButton loading-spinner async in Loading). These are real documentation improvements that also help human readers.💡 Context
Today's MCP server returns mechanical names/values: "give me all components named X", "give me the token with value Y". It can't answer prose-y questions like "what's the difference between
DtButton kind='primary'andkind='danger'?" or "how do I migrate fromDtOldPopover?" — the AI client either hallucinates or punts.This PR closes that gap by exposing the public docs corpus to AI clients via the MCP. The acceptance bar from
project_mcp_search_documentation_scenarios.md(saved memory) lists 10 representative natural-language questions; all 10 now return relevant top-3 results.Part of DLT-3404 (Q2 26 — Dialtone AI Consumability) epic. Blocks DLT-3110 (
search_internal_docsfor the internal corpus) — the internal-corpus sibling waits for this pattern to ship.Architecturally this follows the existing tool pattern exactly: producer (
packages/<source>/) → JSON artifact → static import indialtone-query-core/src/data.ts→ search function indialtone-query-core/src/tools/→ registered in MCP transport + CLI subcommand. No new architectural primitives.📝 Checklist
For all PRs:
.claude/rules/mcp-server.mdto 5 tools / 6 resources; public guide adds bullet forsearch_documentation).📷 Screenshots / GIFs
🔗 Sources
search_internal_docsfor internal corpus)project_mcp_search_documentation_scenarios.md