feat!: explicit Pinecone config, core/alliance split, generic exports - #109
Merged
wpak-ai merged 6 commits intoMay 27, 2026
Merged
Conversation
📝 WalkthroughWalkthroughSplit repository into a generic core bridge and an Alliance app layer; move config defaults into core; make reranking optional with explicit skip signals; add a URL generation registry and Alliance built-ins; rework package exports; update examples, docs, and tests to match the new layout. ChangesCore/Alliance Architecture Refactor
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 #109 +/- ##
=======================================
Coverage ? 81.21%
=======================================
Files ? 38
Lines ? 1299
Branches ? 430
=======================================
Hits ? 1055
Misses ? 242
Partials ? 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli.ts (1)
132-157:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the first CLI example to include the required index.
Line 155 still shows
--api-keyonly, but--index-nameis now required (unless env is set). This example can fail as written.Proposed fix
- pinecone-read-only-mcp --api-key YOUR_KEY + pinecone-read-only-mcp --api-key YOUR_KEY --index-name my-index🤖 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/cli.ts` around lines 132 - 157, The first usage example inside the process.stdout.write usage block still shows only "--api-key YOUR_KEY" but --index-name is now required; update that example to include a sample index name (e.g., "--api-key YOUR_KEY --index-name my-index") and also update the alternate example that uses env export to include --index-name my-index if not relying on PINECONE_INDEX_NAME; edit the string passed to process.stdout.write in src/cli.ts (the Usage/Examples block) so both sample lines show a valid combination including the required --index-name.
🧹 Nitpick comments (2)
src/alliance/setup.ts (1)
18-18: ⚡ Quick winPrefer a single Alliance config entrypoint in setup.
Line 18 re-composes Alliance config logic inline. Prefer routing through the Alliance resolver so future Alliance-specific config behavior stays centralized and doesn’t drift.
🤖 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/alliance/setup.ts` at line 18, The alliance config is being composed inline by calling applyAllianceRerankDefault directly; instead route through the centralized Alliance resolver so all Alliance-specific config logic stays in one place: replace the inline composition that produces allianceConfig (currently using applyAllianceRerankDefault and resolveConfig) with a single call to the Alliance resolver (e.g., resolveAllianceConfig) passing the base config (config ?? resolveConfig({})) so that applyAllianceRerankDefault and any future Alliance adjustments are applied inside that resolver.src/core/index.ts (1)
19-25: ⚡ Quick winExport
resetUrlGenerationRegistryfrom the core barrel.The URL registry export set here omits the reset API, which makes registry state harder to control for consumers using only the package root core entrypoint.
♻️ Proposed fix
export { registerUrlGenerator, unregisterUrlGenerator, + resetUrlGenerationRegistry, generateUrlForNamespace, hasUrlGenerator, } from './server/url-registry.js';🤖 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/index.ts` around lines 19 - 25, The core barrel is missing the resetUrlGenerationRegistry export, so add it to the re-exports in src/core/index.ts (the same place that re-exports registerUrlGenerator, unregisterUrlGenerator, generateUrlForNamespace, hasUrlGenerator) by exporting resetUrlGenerationRegistry from './server/url-registry.js' so consumers can reset registry state via the core entrypoint; also export its type if applicable (e.g., the corresponding type name) alongside the existing UrlGenerationResult/UrlGenerator/UrlGeneratorFn exports.
🤖 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 @.env.example:
- Around line 9-11: The .env.example currently pre-enables the reranker by
setting PINECONE_RERANK_MODEL which contradicts the “unset is OK” note; change
the example so the variable is not set by default (either remove the assignment,
leave it blank, or comment it out) and instead provide the example value
(PINECONE_RERANK_MODEL=bge-reranker-v2-m3) as a commented suggestion or separate
example, so setupCoreServer and CLI/alliance behavior remains optional when the
variable is absent.
In `@examples/custom-url-generator.ts`:
- Around line 46-48: The example currently falls back to demo credentials;
remove the silent defaults and make the required env vars explicit by validating
process.env['PINECONE_API_KEY'] and process.env['PINECONE_INDEX_NAME'] (after
trimming) and throw or exit with a clear error if they are missing; then pass
those validated values to resolveAllianceConfig (referencing the apiKey,
indexName, and resolveAllianceConfig identifiers) so the example fails loudly on
misconfiguration instead of using demo values.
In `@examples/README.md`:
- Line 8: Update the README row for library-embedding-demo.ts to reference
resolveAllianceConfig instead of resolveConfig; edit the description to read
something like "Minimal **library embedding** (`resolveAllianceConfig`,
`setPineconeClient`, `setupAllianceServer`)" so it matches the actual symbols
used in library-embedding-demo.ts and avoids the stale resolveConfig reference.
In `@README.md`:
- Around line 131-143: The README example mixes core and alliance APIs: replace
the core resolveConfig usage with the alliance-specific resolver so the sequence
matches the recommended pattern; update the imports to import
resolveAllianceConfig from '`@will-cppa/pinecone-read-only-mcp/alliance`' (instead
of resolveConfig), call resolveAllianceConfig({ ... }) to produce config used by
setPineconeClient(...) and await setupAllianceServer(config), and ensure the
import list and function call references (resolveAllianceConfig,
setPineconeClient, setupAllianceServer, PineconeClient) are aligned.
In `@src/core/pinecone-client.ts`:
- Around line 39-41: Normalize empty-string rerank model inputs in the Pinecone
client constructor: when setting this.rerankModel from config.rerankModel, treat
'' as undefined (e.g., if config.rerankModel === '' then set this.rerankModel =
undefined) so downstream checks (the presence check around rerank usage and the
logic that sets rerank_skipped_reason) behave consistently; update only the
constructor assignment where this.rerankModel is set to ensure blank inputs are
normalized before any methods reference it.
In `@src/core/server/url-registry.ts`:
- Around line 70-76: Registration trims the namespace before storing it, but
unregisterUrlGenerator(namespace) and hasUrlGenerator(namespace) (and the URL
lookup/generation call referenced around the later pair at lines 92-93) use the
raw input causing mismatches; normalize the namespace input consistently by
trimming (e.g., call .trim()) in unregisterUrlGenerator, hasUrlGenerator, and
the URL lookup/generation function so all paths use the same normalized key as
registerUrlGenerator.
---
Outside diff comments:
In `@src/cli.ts`:
- Around line 132-157: The first usage example inside the process.stdout.write
usage block still shows only "--api-key YOUR_KEY" but --index-name is now
required; update that example to include a sample index name (e.g., "--api-key
YOUR_KEY --index-name my-index") and also update the alternate example that uses
env export to include --index-name my-index if not relying on
PINECONE_INDEX_NAME; edit the string passed to process.stdout.write in
src/cli.ts (the Usage/Examples block) so both sample lines show a valid
combination including the required --index-name.
---
Nitpick comments:
In `@src/alliance/setup.ts`:
- Line 18: The alliance config is being composed inline by calling
applyAllianceRerankDefault directly; instead route through the centralized
Alliance resolver so all Alliance-specific config logic stays in one place:
replace the inline composition that produces allianceConfig (currently using
applyAllianceRerankDefault and resolveConfig) with a single call to the Alliance
resolver (e.g., resolveAllianceConfig) passing the base config (config ??
resolveConfig({})) so that applyAllianceRerankDefault and any future Alliance
adjustments are applied inside that resolver.
In `@src/core/index.ts`:
- Around line 19-25: The core barrel is missing the resetUrlGenerationRegistry
export, so add it to the re-exports in src/core/index.ts (the same place that
re-exports registerUrlGenerator, unregisterUrlGenerator,
generateUrlForNamespace, hasUrlGenerator) by exporting
resetUrlGenerationRegistry from './server/url-registry.js' so consumers can
reset registry state via the core entrypoint; also export its type if applicable
(e.g., the corresponding type name) alongside the existing
UrlGenerationResult/UrlGenerator/UrlGeneratorFn exports.
🪄 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: 0cf9106e-4108-42a8-ba1d-a40a95616370
📒 Files selected for processing (81)
.env.exampleCHANGELOG.mdREADME.mddocs/CONFIGURATION.mdexamples/README.mdexamples/custom-url-generator.tsexamples/demo-mock-pinecone-client.tsexamples/guided-query-demo.tsexamples/library-embedding-demo.tsexamples/suggest-flow-demo.tsexamples/tsconfig.jsonpackage.jsonscripts/test-search.tssrc/alliance/config.test.tssrc/alliance/config.tssrc/alliance/index.tssrc/alliance/setup.tssrc/alliance/tools/guided-query-tool.test.tssrc/alliance/tools/guided-query-tool.tssrc/alliance/tools/suggest-query-params-tool.test.tssrc/alliance/tools/suggest-query-params-tool.tssrc/alliance/url-builtins.test.tssrc/alliance/url-builtins.tssrc/cli.tssrc/constants.tssrc/core/config.test.tssrc/core/config.tssrc/core/index.tssrc/core/pinecone-client.test.tssrc/core/pinecone-client.tssrc/core/pinecone/indexes.test.tssrc/core/pinecone/indexes.tssrc/core/pinecone/rerank.test.tssrc/core/pinecone/rerank.tssrc/core/pinecone/search.test.tssrc/core/pinecone/search.tssrc/core/rerank-trace.test.tssrc/core/rerank-trace.tssrc/core/server.test.tssrc/core/server/client-context.tssrc/core/server/config-context.tssrc/core/server/format-query-result.test.tssrc/core/server/format-query-result.tssrc/core/server/metadata-filter.test.tssrc/core/server/metadata-filter.tssrc/core/server/namespace-router.tssrc/core/server/namespace-utils.tssrc/core/server/namespaces-cache.tssrc/core/server/query-suggestion.tssrc/core/server/reassemble-documents.test.tssrc/core/server/reassemble-documents.tssrc/core/server/retry.test.tssrc/core/server/retry.tssrc/core/server/suggestion-flow.tssrc/core/server/tool-error.test.tssrc/core/server/tool-error.tssrc/core/server/tool-response.tssrc/core/server/tools/count-tool.test.tssrc/core/server/tools/count-tool.tssrc/core/server/tools/generate-urls-tool.test.tssrc/core/server/tools/generate-urls-tool.tssrc/core/server/tools/keyword-search-tool.test.tssrc/core/server/tools/keyword-search-tool.tssrc/core/server/tools/list-namespaces-tool.test.tssrc/core/server/tools/list-namespaces-tool.tssrc/core/server/tools/namespace-router-tool.test.tssrc/core/server/tools/namespace-router-tool.tssrc/core/server/tools/query-documents-tool.test.tssrc/core/server/tools/query-documents-tool.tssrc/core/server/tools/query-tool.test.tssrc/core/server/tools/query-tool.tssrc/core/server/tools/test-helpers.tssrc/core/server/url-registry.test.tssrc/core/server/url-registry.tssrc/core/setup.tssrc/index.tssrc/logger.tssrc/server.tssrc/server/url-generation.test.tssrc/server/url-generation.tssrc/types.ts
💤 Files with no reviewable changes (3)
- src/server/url-generation.test.ts
- src/server/url-generation.ts
- src/server.ts
AuraMindNest
approved these changes
May 26, 2026
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@README.md`:
- Around line 96-107: The docs contradict whether reranking is enabled by
default: reconcile the behavior for PINECONE_RERANK_MODEL and the CLI flag
--rerank-model so both sections say the same thing; pick the canonical behavior
(either default to bge-reranker-v2-m3 when unset, or treat omission as disabling
rerank), then update the Quick reference row for PINECONE_RERANK_MODEL and the
CLI docs for --rerank-model to reflect that choice, and add a short note showing
how to explicitly disable reranking (e.g. using a special value like "none" or a
dedicated flag) so operators know how to turn reranking off. Ensure both places
reference PINECONE_RERANK_MODEL and --rerank-model consistently.
In `@src/alliance/config.ts`:
- Around line 2-3: resolveAllianceConfig currently aliases straight to
resolveConfig and thus injects the default rerank model even when the env var
PINECONE_RERANK_MODEL is unset; change resolveAllianceConfig so it does not
force a default value—call resolveConfig or build the returned config such that
rerank model remains undefined when process.env.PINECONE_RERANK_MODEL is not set
(do not substitute 'bge-reranker-v2-m3' yourself), i.e., detect
PINECONE_RERANK_MODEL and only set rerankModel when present so unset remains
opt-out.
In `@src/core/config.ts`:
- Around line 30-31: The config currently forces DEFAULT_RERANK_MODEL into
rerankModel during resolveConfig()/config construction which makes reranking
always enabled; change the logic so that if the env var PINECONE_RERANK_MODEL is
unset or blank we leave rerankModel undefined (or empty) instead of assigning
DEFAULT_RERANK_MODEL, thereby restoring the "disabled when unset" behavior.
Update the resolver that sets rerankModel (reference symbol: resolveConfig /
rerankModel / DEFAULT_RERANK_MODEL) to only assign DEFAULT_RERANK_MODEL when a
specific "enable" flag or explicit instruction is present (or when callers
explicitly request default), and adjust the config type/consumers if needed to
accept undefined/'' as the skip/no_model path.
🪄 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: 3ac903bf-475b-4ea5-92d3-3e50f0492e76
📒 Files selected for processing (24)
.env.exampleCHANGELOG.mdREADME.mddocs/CONFIGURATION.mdexamples/README.mdexamples/custom-url-generator.tsexamples/guided-query-demo.tsexamples/library-embedding-demo.tsexamples/suggest-flow-demo.tssrc/alliance/config.test.tssrc/alliance/config.tssrc/alliance/setup.tssrc/cli.tssrc/core/config.test.tssrc/core/config.tssrc/core/index.tssrc/core/pinecone-client.tssrc/core/server/tools/query-documents-tool.tssrc/core/server/tools/query-tool.test.tssrc/core/server/tools/query-tool.tssrc/core/server/url-registry.tssrc/core/setup.tssrc/index.tssrc/types.ts
💤 Files with no reviewable changes (1)
- src/core/setup.ts
✅ Files skipped from review due to trivial changes (2)
- examples/README.md
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (8)
- src/core/server/tools/query-tool.ts
- src/core/config.test.ts
- src/core/index.ts
- examples/suggest-flow-demo.ts
- examples/library-embedding-demo.ts
- docs/CONFIGURATION.md
- src/core/pinecone-client.ts
- src/core/server/url-registry.ts
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 `@src/core/config.ts`:
- Around line 118-120: The code currently assigns indexName with a fallback to
DEFAULT_INDEX_NAME; change this to require an explicit value instead: obtain the
candidate from overrides.indexName or env['PINECONE_INDEX_NAME'] using
trimOptional and if the result is null/empty, throw a clear configuration error
(or return a failure) rather than defaulting to DEFAULT_INDEX_NAME so callers of
this module cannot silently use "rag-hybrid"; adjust the logic around the
indexName variable (referencing overrides.indexName, env['PINECONE_INDEX_NAME'],
trimOptional, and DEFAULT_INDEX_NAME) to enforce and surface a missing-index
configuration error.
🪄 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: b6055e7a-432a-4783-bc0d-213e66f330d8
📒 Files selected for processing (10)
.env.exampleCHANGELOG.mdREADME.mddocs/CONFIGURATION.mdexamples/README.mdsrc/cli.tssrc/core/config.test.tssrc/core/config.tssrc/core/index.tssrc/types.ts
✅ Files skipped from review due to trivial changes (5)
- .env.example
- CHANGELOG.md
- examples/README.md
- src/core/index.ts
- docs/CONFIGURATION.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types.ts
wpak-ai
approved these changes
May 27, 2026
This was referenced May 27, 2026
6 tasks
This was referenced Jun 1, 2026
This was referenced Jun 11, 2026
3 tasks
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
3 participants
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.
Summary
Centralizes Pinecone configuration in
resolveConfig(), splits the codebase into core (generic MCP–Pinecone bridge) and alliance (Alliance app tools), and limits the default npm export to the core surface. Alliance MCP configs that only setPINECONE_API_KEYkeep working via documented defaults (rag-hybrid,bge-reranker-v2-m3).Changes
Configuration (#104)
parseCli()→resolveConfig()→PineconeClient/setupAllianceServer(noprocess.envreads in the client).PINECONE_API_KEY(or--api-key); clear error if missing.src/core/config.tsonly — removed fromsrc/constants.ts):PINECONE_INDEX_NAME/--index-name→ defaultrag-hybrid(DEFAULT_INDEX_NAME)PINECONE_RERANK_MODEL/--rerank-model→ defaultbge-reranker-v2-m3(DEFAULT_RERANK_MODEL)query/query_documentsforwardrerank_skipped_reason: no_modelwhen rerank was requested butPineconeClienthas no model (manual library embed);guided_queryexposesdecision_trace.rerank_status: skipped_no_model.Architecture (#105)
src/core/—setupCoreServer,resolveConfig,PineconeClient, seven generic MCP tools.src/alliance/—setupAllianceServer,suggest_query_params,guided_query, Boost/Slack URL builtins.".") exports core only.Exports (#107)
registerUrlGenerator, etc.).mailing/slack-Cpplanggenerators on@will-cppa/pinecone-read-only-mcp/alliance.@will-cppa/pinecone-read-only-mcp/alliance.Breaking changes
setupServer, monolithicsrc/server.tssetupCoreServerfrom package root; full server via/allianceconstants.ts+ env reads in clientresolveConfig()insrc/core/config.ts; explicitDEFAULT_*constantsimport { setupServer } from '…'import { setupAllianceServer } from '…/alliance'PINECONE_API_KEYrequired; index/rerank default when unset (override with env)Test plan
npm run ci(typecheck, lint, format, build, test:coverage)npm run smoke(--helpdocuments index/rerank defaults)PINECONE_INDEX_NAME, no--index-name): server starts and stderr showsUsing Pinecone index: rag-hybrid (sparse: rag-hybrid-sparse)and rerank modelPINECONE_INDEX_NAME=my-indexor--index-name my-indexused instead of default@will-cppa/pinecone-read-only-mcpand@will-cppa/pinecone-read-only-mcp/alliance(no stray backticks in README snippets)Note: Semver bump and full migration guide deferred to PR B (
week4/release).Related issues