Skip to content

Commit 423dda2

Browse files
fix: address CodeRabbit feedback (Prettier, response.index, clampTopK, test summary)
fix: address CodeRabbit feedback for keyword_search and tooling - Prettier: format keyword-search-tool.ts and index.ts - response.index: add getSparseIndexName() on PineconeClient; use in keyword_search - pinecone-client: extract clampTopK(), use in query() and keywordSearch() - keyword-search-tool: remove redundant top_k ?? 10 (Zod default) - test-search: add Test 5 timing to performance summary (duration5 / skipped)
1 parent 5b97159 commit 423dda2

4 files changed

Lines changed: 48 additions & 26 deletions

File tree

scripts/test-search.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ async function test() {
153153
}
154154

155155
// Test 5: Keyword (sparse-only) search on pinecone-rag-sparse
156+
let duration5: number | undefined;
157+
let test5Skipped = false;
156158
console.log(`\n🔤 Test 5: Keyword search (sparse-only index)`);
157159
console.log(` Namespace: "${testNamespace}"`);
158160
console.log(` Query: "test query"`);
@@ -164,14 +166,21 @@ async function test() {
164166
namespace: testNamespace,
165167
topK: 3,
166168
});
167-
const duration5 = Date.now() - startTime5;
169+
duration5 = Date.now() - startTime5;
168170
console.log(`✅ Keyword search returned ${results5.length} result(s) in ${duration5}ms`);
169171
if (results5.length > 0) {
170-
console.log(` First result score: ${results5[0].score.toFixed(4)}, reranked: ${results5[0].reranked}`);
172+
console.log(
173+
` First result score: ${results5[0].score.toFixed(4)}, reranked: ${results5[0].reranked}`
174+
);
171175
}
172176
} catch (kwError) {
173-
console.log(`⚠️ Keyword search skipped: ${kwError instanceof Error ? kwError.message : String(kwError)}`);
174-
console.log(` Ensure PINECONE_SPARSE_INDEX_NAME (e.g. pinecone-rag-sparse) exists and has data.`);
177+
test5Skipped = true;
178+
console.log(
179+
`⚠️ Keyword search skipped: ${kwError instanceof Error ? kwError.message : String(kwError)}`
180+
);
181+
console.log(
182+
` Ensure PINECONE_SPARSE_INDEX_NAME (e.g. pinecone-rag-sparse) exists and has data.`
183+
);
175184
}
176185

177186
console.log('\n✨ All tests completed successfully!');
@@ -181,6 +190,11 @@ async function test() {
181190
if (duration3 !== undefined) {
182191
console.log(` With metadata filter: ${duration3}ms`);
183192
}
193+
if (duration5 !== undefined) {
194+
console.log(` Keyword search: ${duration5}ms`);
195+
} else if (test5Skipped) {
196+
console.log(` Keyword search: skipped`);
197+
}
184198
console.log(` Reranking overhead: ${duration2 - duration1}ms`);
185199
} catch (error) {
186200
console.error('\n❌ Error during testing:', error);

src/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@
1111
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js';
1212
import { PineconeClient } from './pinecone-client.js';
1313
import { setupServer, setPineconeClient } from './server.js';
14-
import { DEFAULT_INDEX_NAME, DEFAULT_RERANK_MODEL, DEFAULT_SPARSE_INDEX_NAME } from './constants.js';
14+
import {
15+
DEFAULT_INDEX_NAME,
16+
DEFAULT_RERANK_MODEL,
17+
DEFAULT_SPARSE_INDEX_NAME,
18+
} from './constants.js';
1519
import type { LogLevel } from './config.js';
1620
import { setLogLevel } from './logger.js';
1721
import * as dotenv from 'dotenv';

src/pinecone-client.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,25 @@ export class PineconeClient {
8282
config.defaultTopK || parseInt(process.env['PINECONE_TOP_K'] || String(DEFAULT_TOP_K));
8383
}
8484

85+
/** Returns the configured sparse index name used for keyword search (runtime value). */
86+
getSparseIndexName(): string {
87+
return this.sparseIndexName;
88+
}
89+
90+
/**
91+
* Normalize and clamp topK from request (validates >= 1, caps at MAX_TOP_K).
92+
*/
93+
private clampTopK(requested: number | undefined): number {
94+
let topK = requested !== undefined ? requested : this.defaultTopK;
95+
if (topK < 1) {
96+
throw new Error('topK must be at least 1');
97+
}
98+
if (topK > MAX_TOP_K) {
99+
topK = MAX_TOP_K;
100+
}
101+
return topK;
102+
}
103+
85104
/**
86105
* Ensure Pinecone client is initialized
87106
*/
@@ -413,14 +432,7 @@ export class PineconeClient {
413432
throw new Error('Query cannot be empty');
414433
}
415434

416-
let topK = requestedTopK !== undefined ? requestedTopK : this.defaultTopK;
417-
if (topK < 1) {
418-
throw new Error('topK must be at least 1');
419-
}
420-
421-
if (topK > MAX_TOP_K) {
422-
topK = MAX_TOP_K;
423-
}
435+
const topK = this.clampTopK(requestedTopK);
424436

425437
// When reranking, Pinecone requires chunk_text in returned fields; add it if user specified fields without it
426438
const searchFields =
@@ -494,13 +506,7 @@ export class PineconeClient {
494506
throw new Error('Query cannot be empty');
495507
}
496508

497-
let topK = requestedTopK !== undefined ? requestedTopK : this.defaultTopK;
498-
if (topK < 1) {
499-
throw new Error('topK must be at least 1');
500-
}
501-
if (topK > MAX_TOP_K) {
502-
topK = MAX_TOP_K;
503-
}
509+
const topK = this.clampTopK(requestedTopK);
504510

505511
const keywordIndex = await this.ensureKeywordIndex();
506512
const searchOptions = requestedFields?.length ? { fields: requestedFields } : undefined;

src/server/tools/keyword-search-tool.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
22
import { z } from 'zod';
3-
import { DEFAULT_SPARSE_INDEX_NAME, MAX_TOP_K, MIN_TOP_K } from '../../constants.js';
3+
import { MAX_TOP_K, MIN_TOP_K } from '../../constants.js';
44
import { getPineconeClient } from '../client-context.js';
55
import { formatQueryResultRows } from '../format-query-result.js';
66
import { metadataFilterSchema, validateMetadataFilter } from '../metadata-filter.js';
@@ -67,7 +67,7 @@ async function executeKeywordSearch(params: {
6767
status: 'success',
6868
query: query_text,
6969
namespace,
70-
index: DEFAULT_SPARSE_INDEX_NAME,
70+
index: client.getSparseIndexName(),
7171
metadata_filter: metadata_filter,
7272
result_count: formattedResults.length,
7373
results: formattedResults,
@@ -91,9 +91,7 @@ export function registerKeywordSearchTool(server: McpServer): void {
9191
query_text: z.string().describe('Search query text (keyword/lexical match).'),
9292
namespace: z
9393
.string()
94-
.describe(
95-
'Namespace to search. Use list_namespaces to discover available namespaces.'
96-
),
94+
.describe('Namespace to search. Use list_namespaces to discover available namespaces.'),
9795
top_k: z
9896
.number()
9997
.int()
@@ -117,7 +115,7 @@ export function registerKeywordSearchTool(server: McpServer): void {
117115
const response = await executeKeywordSearch({
118116
query_text: params.query_text,
119117
namespace: params.namespace,
120-
top_k: params.top_k ?? 10,
118+
top_k: params.top_k,
121119
metadata_filter: params.metadata_filter,
122120
fields: params.fields,
123121
});

0 commit comments

Comments
 (0)