Load per-source schema manifests from Pinecone _mcp_config at startup - #210
Conversation
📝 WalkthroughWalkthroughJSON configuration parsing now validates source metadata and environment indirection. Startup can load missing descriptions and namespace schemas from ChangesSource configuration parsing and controls
Record retrieval and remote schema loading
Startup and namespace discovery wiring
Configuration and operational documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Startup
participant PineconeClient
participant RemoteSchemaLoader
participant ServerContext
Startup->>PineconeClient: Fetch _mcp_config/schema_manifest
PineconeClient->>RemoteSchemaLoader: Return record fields
RemoteSchemaLoader-->>Startup: Return merged definitions and warnings
Startup->>ServerContext: Provide resolved sources or declared namespaces
ServerContext-->>Startup: Discover namespaces using declared metadata
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #210 +/- ##
=======================================
Coverage ? 85.61%
=======================================
Files ? 47
Lines ? 2488
Branches ? 857
=======================================
Hits ? 2130
Misses ? 357
Partials ? 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 41 minutes. |
|
@coderabbitai full review again |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 29 minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/pinecone/indexes.ts (1)
237-253: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider reusing the cached
denseIndexinstead of a freshpc.index()call.Every other method in this class (
ensureIndexes,listNamespacesWithMetadata,checkIndexes) reuses the cacheddenseIndex/sparseIndexfromensureIndexes(). This method instead callspc.index(this.indexName)fresh viaensureClient(). Per Pinecone's own guidance, targeting an index by name can trigger an internaldescribeIndex()call to resolve the host, and the SDK FAQ recommends reusing resolved index instances to avoid that extra round-trip. Routing throughensureIndexes()and destructuringdenseIndexwould align with the rest of the class and avoid a possibly redundant resolution call on every invocation — worth verifyingSearchableIndexexposes.fetch()before applying.♻️ Possible direction (verify `SearchableIndex.fetch` exists before applying)
async fetchRecordFields(namespace: string, id: string): Promise<Record<string, unknown> | null> { - const pc = this.ensureClient(); - const response = await pc.index(this.indexName).fetch({ ids: [id], namespace }); + const { denseIndex } = await this.ensureIndexes(); + const response = await denseIndex.fetch({ ids: [id], namespace });🤖 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 `@src/core/pinecone/indexes.ts` around lines 237 - 253, Update fetchRecordFields to obtain and reuse the cached denseIndex from ensureIndexes() instead of calling ensureClient().index(this.indexName) for each request. Verify that SearchableIndex exposes fetch(), then invoke fetch on the resolved denseIndex while preserving the existing namespace, ID, metadata merge, and null-result behavior.
🤖 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 `@src/core/pinecone/indexes.ts`:
- Around line 237-253: Update fetchRecordFields() to perform the Pinecone fetch
with a bounded AbortSignal-backed timeout, ensuring slow or hung remote requests
reject so loadRemoteSchemaForSource() can reach its existing warning path and
startup is not blocked.
---
Nitpick comments:
In `@src/core/pinecone/indexes.ts`:
- Around line 237-253: Update fetchRecordFields to obtain and reuse the cached
denseIndex from ensureIndexes() instead of calling
ensureClient().index(this.indexName) for each request. Verify that
SearchableIndex exposes fetch(), then invoke fetch on the resolved denseIndex
while preserving the existing namespace, ID, metadata merge, and null-result
behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 906e29da-9a72-495d-b53f-f025163669de
📒 Files selected for processing (15)
CHANGELOG.mddocs/CONFIGURATION.mddocs/SECURITY.mdsrc/cli.tssrc/core/config.tssrc/core/pinecone-client.tssrc/core/pinecone/indexes.test.tssrc/core/pinecone/indexes.tssrc/core/server/remote-schema.test.tssrc/core/server/remote-schema.tssrc/core/server/server-context.composition.test.tssrc/core/server/server-context.tssrc/core/server/source-config.test.tssrc/core/server/source-config.tssrc/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
jonathanMLDev
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/CONFIGURATION.md`:
- Around line 237-240: Update the remote schema loading guidance for both
single-key and multi-source embedding flows to require !config.checkIndexes in
addition to !config.disableRemoteSchema, matching the CLI guard and preventing
remote fetches during index-check mode.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e94933a-38ab-4673-bfb4-5fb186aaba6f
📒 Files selected for processing (9)
docs/CONFIGURATION.mdsrc/cli.test.tssrc/core/pinecone-client.test.tssrc/core/pinecone-client.tssrc/core/pinecone/indexes.test.tssrc/core/pinecone/indexes.tssrc/core/server/remote-schema.test.tssrc/core/server/remote-schema.tssrc/core/server/source-config.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/core/pinecone/indexes.ts
- src/core/pinecone-client.ts
- src/core/server/source-config.test.ts
- src/core/server/remote-schema.test.ts
- src/core/server/remote-schema.ts
- src/core/pinecone/indexes.test.ts
3 participants
Summary
descriptionand namespace declarations automatically from the reserved Pinecone_mcp_confignamespace (schema_manifestrecord) at startup, for both multi-source and single-key deployments.PINECONE_CONFIG_FILE/--config-filefor local overrides; localnamespacesalways win over remote manifests.PINECONE_DISABLE_REMOTE_SCHEMA/--disable-remote-schemato opt out of remote loading.fetchRecordFieldsso manifestchunk_textis readable on integrated-embedding indexes.Motivation
MCP hosts like Cursor only accept string
envvalues, so embedding rich per-source config inline inmcp.jsonis awkward. Storing schema manifests in Pinecone keepsmcp.jsonlimited to API keys and index names while still providing declared descriptions andmetadata_schemafor tools likelist_namespaces.Configuration precedence
PINECONE_CONFIG_FILE(local file)PINECONE_SOURCES/--sources_mcp_configmanifest (when no localnamespacesare declared)Inline JSON in
PINECONE_SOURCESis not supported; a clear startup error points users toPINECONE_CONFIG_FILEor_mcp_configinstead.Test plan
npm testnpm run lintnpm run format:checknpm run buildpinecone-search-localin Cursor and confirm startup log:Loading _mcp_config schema manifest for N source(s)...list_namespacesand verifyschema_source: "declared"for namespaces covered by the manifestPINECONE_DISABLE_REMOTE_SCHEMA=true, reload, and confirm remote loading is skippedPINECONE_CONFIG_FILElocalnamespacesstill override remote manifestsSummary by CodeRabbit
New Features
Improvements