Skip to content

Commit bd94c14

Browse files
authored
Register suggest_query_params in core (#223)
* feat: register suggest_query_params in core (#221) Move the suggest_query_params tool from the Alliance overlay into core (src/core/server/tools/) and register it in setupCoreServerOnContext, so a core-initialized server can expose it. The tool only imported from core/server, so this is a clean move; Alliance still gets it via the core setup it calls, and its now-duplicate registration is removed. Deliberately narrow per the issue: no change to the disableSuggestFlow default (core stays gate-off, guided_query first) or to CORE_SERVER_INSTRUCTIONS, and Alliance behaviour is unchanged. Adds a core-registration test and updates the RC-readiness core tool list. * test: execute suggest_query_params on a core server end-to-end (#221 review) The cross-entry-point test proves core setup invokes the registrar; add a callTool round-trip in the RC-readiness harness so a core-initialized server actually exposes and executes suggest_query_params (asserts status success + namespace_found). A broken registration can no longer pass by only being listed. * docs: reflect suggest_query_params as a core tool (#221 review) suggest_query_params is now registered in core, so update the docs that still called it Alliance-specific: the setupCoreServer docblock (nine core tools, ten with multi-source list_sources; tool registered but the suggest-flow gate stays off by default for core), the CORE_SERVER_INSTRUCTIONS docstring and the setupAllianceServer docblock, and the Core-vs-Alliance table + gate paragraph in TOOLS.md. Alliance's Boost/Slack URL generators are clarified as generate_urls registry entries, not a separate MCP tool. PACKAGE_SPLIT_EVAL.md left as a point-in-time eval. * docs(suggest_query_params): qualify the mandatory-call note by gate state The tool description said the call is mandatory before query/count tools unconditionally, but core registers the tool with the suggest-flow gate off by default. Say it's required only when the gate is enabled (Alliance default) and optional otherwise, matching the gate paragraph in TOOLS.md.
1 parent cc4e057 commit bd94c14

10 files changed

Lines changed: 75 additions & 31 deletions

docs/TOOLS.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ Promotion process: [deprecation-policy.md § Stable vs experimental](./deprecati
2727

2828
| Setup | Tools | MCP instructions |
2929
| ----- | ----- | ------------------ |
30-
| `setupCoreServer` (package root) | **8** (single-key) or **9** (multi-source adds `list_sources`): `list_namespaces`, `namespace_router`, `count`, `query`, `keyword_search`, `query_documents`, `generate_urls`, `guided_query` | `CORE_SERVER_INSTRUCTIONS` — includes `guided_query`; no `suggest_query_params` |
31-
| `setupAllianceServer` / published CLI | **9** (single-key) or **10** (multi-source): core tools plus `suggest_query_params` | `ALLIANCE_SERVER_INSTRUCTIONS` — includes suggest-flow quickstart |
30+
| `setupCoreServer` (package root) | **9** (single-key) or **10** (multi-source adds `list_sources`): `list_namespaces`, `namespace_router`, `count`, `query`, `keyword_search`, `query_documents`, `generate_urls`, `guided_query`, `suggest_query_params` | `CORE_SERVER_INSTRUCTIONS` — includes `guided_query`; registers `suggest_query_params` but the suggest-flow gate is off by default |
31+
| `setupAllianceServer` / published CLI | **9** (single-key) or **10** (multi-source): the same core tool surface — Alliance adds built-in Boost/Slack URL generators for `generate_urls` (not an extra MCP tool) and enables the gate by default | `ALLIANCE_SERVER_INSTRUCTIONS` — includes suggest-flow quickstart |
3232

3333
## Suggest-flow gate
3434

3535
When **`disableSuggestFlow`** is **`false`** (Alliance default via `resolveAllianceConfig` / CLI), tools **`query`**, **`count`**, and **`query_documents`** require a prior successful **`suggest_query_params`** call for the **same namespace string** within the cache TTL (see `PINECONE_CACHE_TTL_SECONDS`). The gate is in-process memory (`requireSuggested`).
3636

37-
When **`disableSuggestFlow`** is **`true`** (core default via `resolveConfig`), the gate is bypassed — suitable for `setupCoreServer` embedders that do not register `suggest_query_params`.
37+
When **`disableSuggestFlow`** is **`true`** (core default via `resolveConfig`), the gate is bypassed: `setupCoreServer` still registers `suggest_query_params`, but embedders can call it optionally rather than being required to before `query` / `count` / `query_documents`.
3838

3939
**Namespace consistency:** use the **exact same** `namespace` value (including trimming — avoid leading/trailing spaces in one call and not the other) for `suggest_query_params` and downstream gated tools. Mismatches yield `FLOW_GATE` with a suggestion to call `suggest_query_params` first.
4040

src/__tests__/cross-entry-point.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { setPineconeClient } from '../core/index.js';
55
import { resolveConfig } from '../core/config.js';
66
import { getDefaultServerContext } from '../core/server/server-context.js';
77
import * as guidedQueryTool from '../core/server/tools/guided-query-tool.js';
8+
import * as suggestQueryParamsTool from '../core/server/tools/suggest-query-params-tool.js';
89
import { registerQueryTool } from '../core/server/tools/query-tool.js';
910
import {
1011
assertToolErrorCode,
@@ -119,4 +120,22 @@ describe('cross-entry-point: core vs Alliance defaults', () => {
119120
expect(ctx.hasToolsRegistered()).toBe(true);
120121
registerSpy.mockRestore();
121122
});
123+
124+
it('core setup registers suggest_query_params handler (#221)', async () => {
125+
const cfg = resolveConfig({ apiKey: 'sk-cross', indexName: 'test-index' });
126+
const ctx = createTestServerContext({
127+
config: cfg,
128+
client: new PineconeClient({
129+
apiKey: cfg.apiKey,
130+
indexName: cfg.indexName,
131+
defaultTopK: cfg.defaultTopK,
132+
}),
133+
});
134+
const registerSpy = vi.spyOn(suggestQueryParamsTool, 'registerSuggestQueryParamsTool');
135+
136+
await setupCoreServer({ context: ctx });
137+
138+
expect(registerSpy).toHaveBeenCalledOnce();
139+
registerSpy.mockRestore();
140+
});
122141
});

src/__tests__/mcp-rc-readiness.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const CORE_TOOLS = [
3535
'query_documents',
3636
'generate_urls',
3737
'guided_query',
38+
'suggest_query_params',
3839
].sort();
3940

4041
/** A fresh, isolated core server (own context + mock client) so several can coexist. */
@@ -158,6 +159,25 @@ describe('MCP RC-readiness harness (#202)', () => {
158159
await client.close();
159160
});
160161

162+
it('core server executes suggest_query_params over the transport (#221)', async () => {
163+
// Proves the tool is not just listed but callable on a core-initialized
164+
// server, so a broken registration can't pass by only being enumerated.
165+
const server = await freshCoreServer(['wg21']);
166+
const client = await connectClient(server);
167+
168+
const res = await client.callTool({
169+
name: 'suggest_query_params',
170+
arguments: { namespace: 'wg21', user_query: 'list papers' },
171+
});
172+
expect(res.isError ?? false).toBe(false);
173+
const text = (res.content as Array<{ type: string; text: string }>)[0]?.text ?? '';
174+
const body = JSON.parse(text) as { status?: string; namespace_found?: boolean };
175+
expect(body.status).toBe('success');
176+
expect(body.namespace_found).toBe(true);
177+
178+
await client.close();
179+
});
180+
161181
it('negotiates the SDK latest protocol version and falls back for an unknown request', async () => {
162182
// A server connects to one transport for its lifetime, so use a fresh one per probe.
163183
const negotiated = await rawInitialize(await freshCoreServer(), LATEST_PROTOCOL_VERSION);

src/alliance/setup.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { createServer, type AllianceServerContext } from '../core/server/server-
55
import { resolveAllianceConfig } from './config.js';
66
import { setupCoreServerOnContext, type ServerHandle } from '../core/setup.js';
77
import { registerBuiltinUrlGenerators } from './url-builtins.js';
8-
import { registerSuggestQueryParamsTool } from './tools/suggest-query-params-tool.js';
98

109
/**
1110
* Options for {@link setupAllianceServer}.
@@ -68,8 +67,10 @@ function normalizeSetupAllianceArgs(
6867
}
6968

7069
/**
71-
* Create and configure the MCP server with the full Alliance tool surface:
72-
* all core tools (including `guided_query`) plus `suggest_query_params` and built-in URL generators.
70+
* Create and configure the MCP server with the full Alliance tool surface: the same
71+
* core tools (including `guided_query` and `suggest_query_params`) plus built-in
72+
* Boost/Slack URL generators wired into `generate_urls`, and the suggest-flow gate
73+
* enabled by default.
7374
*
7475
* When `config` is omitted, resolves env via {@link resolveAllianceConfig} (Alliance index/rerank defaults when unset).
7576
*/
@@ -114,6 +115,5 @@ export async function setupAllianceServer(
114115
}
115116

116117
registerBuiltinUrlGenerators(resolvedCtx);
117-
registerSuggestQueryParamsTool(server, resolvedCtx);
118118
return server;
119119
}

src/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ Features:
4949
Notes:
5050
- Result rows include both \`document_id\` (canonical) and \`paper_number\` (deprecated alias kept for one minor cycle; will be removed in the next major release). Prefer \`document_id\` in new code.`;
5151

52-
/** MCP instructions for {@link setupCoreServer} (eight core tools including guided_query). */
52+
/** MCP instructions for {@link setupCoreServer} (nine core tools including guided_query and suggest_query_params). */
5353
export const CORE_SERVER_INSTRUCTIONS = `Quickstart for AI clients: for most user questions, call \`guided_query\` with the user's question — it does namespace routing, suggestion, and execution in one shot and returns \`experimental.decision_trace\` you can show the user. Alternatively call \`list_namespaces\` to discover namespaces, optionally \`namespace_router\` to rank candidates from user intent, then \`query\` (preset fast/detailed/full), \`count\`, \`query_documents\`, \`keyword_search\`, or \`generate_urls\` as needed.
5454
5555
${SERVER_FEATURES_AND_NOTES}

src/core/server/tools/guided-query-tool.context.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest';
22
import { registerBuiltinUrlGenerators } from '../../../alliance/url-builtins.js';
33
import { guidedQueryResponseSchema } from '../response-schemas.js';
44
import { registerQueryTool } from './query-tool.js';
5-
import { registerSuggestQueryParamsTool } from '../../../alliance/tools/suggest-query-params-tool.js';
5+
import { registerSuggestQueryParamsTool } from './suggest-query-params-tool.js';
66
import { registerGuidedQueryTool } from './guided-query-tool.js';
77
import {
88
assertToolErrorCode,

src/alliance/tools/suggest-query-params-tool.context.test.ts renamed to src/core/server/tools/suggest-query-params-tool.context.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { afterEach, describe, expect, it, vi } from 'vitest';
2-
import { registerQueryTool } from '../../core/server/tools/query-tool.js';
2+
import { registerQueryTool } from './query-tool.js';
33
import { registerSuggestQueryParamsTool } from './suggest-query-params-tool.js';
4-
import { suggestQueryParamsResponseSchema } from '../../core/server/response-schemas.js';
4+
import { suggestQueryParamsResponseSchema } from '../response-schemas.js';
55
import {
66
createMockServer,
77
createTestServerContext,
88
expectMatchesResponseSchema,
99
makeHybridQueryResult,
1010
mockNamespacesWithMetadataResult,
1111
parseToolJson,
12-
} from '../../core/server/tools/test-helpers.js';
12+
} from './test-helpers.js';
1313

1414
const namespaceMetadata = {
1515
document_number: 'string',

src/alliance/tools/suggest-query-params-tool.test.ts renamed to src/core/server/tools/suggest-query-params-tool.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
import { beforeEach, describe, expect, it, vi } from 'vitest';
2-
import { getNamespacesWithCache } from '../../core/server/namespaces-cache.js';
3-
import { markSuggested } from '../../core/server/suggestion-flow.js';
2+
import { getNamespacesWithCache } from '../namespaces-cache.js';
3+
import { markSuggested } from '../suggestion-flow.js';
44
import { registerSuggestQueryParamsTool } from './suggest-query-params-tool.js';
55
import {
66
createMockServer,
77
makeNamespaceCacheEntry,
88
parseToolJson,
99
assertToolErrorCode,
10-
} from '../../core/server/tools/test-helpers.js';
10+
} from './test-helpers.js';
1111

12-
vi.mock('../../core/server/namespaces-cache.js', () => ({
12+
vi.mock('../namespaces-cache.js', () => ({
1313
getNamespacesWithCache: vi.fn(),
1414
}));
1515

16-
vi.mock('../../core/server/suggestion-flow.js', () => ({
16+
vi.mock('../suggestion-flow.js', () => ({
1717
markSuggested: vi.fn(),
1818
}));
1919

src/alliance/tools/suggest-query-params-tool.ts renamed to src/core/server/tools/suggest-query-params-tool.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
22
import { z } from 'zod';
3-
import { normalizeNamespace } from '../../core/server/namespace-utils.js';
4-
import { getNamespacesWithCache } from '../../core/server/namespaces-cache.js';
5-
import { suggestQueryParams } from '../../core/server/query-suggestion.js';
6-
import type { ServerContext } from '../../core/server/server-context.js';
7-
import { markSuggested } from '../../core/server/suggestion-flow.js';
3+
import { normalizeNamespace } from '../namespace-utils.js';
4+
import { getNamespacesWithCache } from '../namespaces-cache.js';
5+
import { suggestQueryParams } from '../query-suggestion.js';
6+
import type { ServerContext } from '../server-context.js';
7+
import { markSuggested } from '../suggestion-flow.js';
88
import {
99
optionalSourceField,
1010
rejectSourceWithoutContext,
1111
resolveSourceForTool,
1212
sourceParamSchema,
1313
sourceValidationError,
14-
} from '../../core/server/source-tool-utils.js';
14+
} from '../source-tool-utils.js';
1515
import {
1616
classifyToolCatchError,
1717
lifecycleToolError,
1818
logToolError,
1919
validationToolError,
20-
} from '../../core/server/tool-error.js';
20+
} from '../tool-error.js';
2121
import {
2222
suggestQueryParamsResponseSchema,
2323
type SuggestQueryParamsResponse,
24-
} from '../../core/server/response-schemas.js';
25-
import { jsonErrorResponse, validatedJsonResponse } from '../../core/server/tool-response.js';
24+
} from '../response-schemas.js';
25+
import { jsonErrorResponse, validatedJsonResponse } from '../tool-response.js';
2626

2727
/** Register the suggest_query_params tool on the MCP server. */
2828
export function registerSuggestQueryParamsTool(server: McpServer, ctx?: ServerContext): void {
@@ -33,7 +33,7 @@ export function registerSuggestQueryParamsTool(server: McpServer, ctx?: ServerCo
3333
"Suggest which fields to request and whether to use the count tool, based on the namespace schema (from list_namespaces) and the user's natural language query. " +
3434
'Call list_namespaces first to get available namespaces and metadata fields. Then call this tool with the target namespace and the user query; ' +
3535
'it returns suggested_fields (only fields that exist in that namespace), use_count_tool (true if the query is a count question), recommended_tool (count | fast | detailed | full — same vocabulary as the query tool preset), and an explanation. ' +
36-
'This step is mandatory before query/count tools; use the returned suggested_fields in query tools to reduce payload and cost.',
36+
'When the suggest-flow gate is enabled (the Alliance default) this call is required before the query/count tools; when the gate is off (the core default) it is optional. Either way, use the returned suggested_fields in query tools to reduce payload and cost.',
3737
inputSchema: {
3838
namespace: z
3939
.string()

src/core/setup.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { registerKeywordSearchTool } from './server/tools/keyword-search-tool.js
1919
import { registerListNamespacesTool } from './server/tools/list-namespaces-tool.js';
2020
import { registerNamespaceRouterTool } from './server/tools/namespace-router-tool.js';
2121
import { registerQueryDocumentsTool } from './server/tools/query-documents-tool.js';
22+
import { registerSuggestQueryParamsTool } from './server/tools/suggest-query-params-tool.js';
2223
import { registerQueryTool } from './server/tools/query-tool.js';
2324

2425
/** MCP server handle with automatic teardown via `await using`. */
@@ -36,10 +37,13 @@ export function teardownServer(): void {
3637
/**
3738
* Create and configure the MCP server with generic (core) tools.
3839
*
39-
* Registers eight core tools including `guided_query`. Does not register Alliance-specific
40-
* `suggest_query_params` or built-in Boost/Slack URL generators. Use
41-
* {@link setupAllianceServer} from `@will-cppa/pinecone-read-only-mcp/alliance` for the
42-
* full Alliance tool surface (suggest-flow gate on by default).
40+
* Registers nine core tools including `guided_query` and `suggest_query_params` (ten
41+
* with multi-source `list_sources`). `suggest_query_params` is registered, but the
42+
* suggest-flow gate stays off by default for core (`disableSuggestFlow: true`), so core
43+
* consumers can query without calling it first. Alliance additionally wires built-in
44+
* Boost/Slack URL generators (registered into the `generate_urls` registry, not a
45+
* separate MCP tool) and enables the gate by default; use {@link setupAllianceServer}
46+
* from `@will-cppa/pinecone-read-only-mcp/alliance` for that.
4347
*/
4448
export type SetupCoreServerOptions = {
4549
config?: CoreServerConfig;
@@ -164,6 +168,7 @@ async function registerCoreToolSurface(
164168
registerQueryDocumentsTool(server, ctx);
165169
registerGenerateUrlsTool(server, ctx);
166170
registerGuidedQueryTool(server, ctx);
171+
registerSuggestQueryParamsTool(server, ctx);
167172

168173
ctx.getConfig();
169174
if (ctx.isMultiSource()) {

0 commit comments

Comments
 (0)