Skip to content

fix(embeddings): allow custom OpenAI embedding path#467

Open
honor2030 wants to merge 1 commit into
rohitg00:mainfrom
honor2030:fix/openai-embedding-path
Open

fix(embeddings): allow custom OpenAI embedding path#467
honor2030 wants to merge 1 commit into
rohitg00:mainfrom
honor2030:fix/openai-embedding-path

Conversation

@honor2030
Copy link
Copy Markdown
Contributor

@honor2030 honor2030 commented May 17, 2026

Summary

  • Adds OPENAI_EMBEDDING_PATH so OpenAI-compatible embedding providers can use custom embedding routes without duplicating /v1.
  • Trims trailing slashes from OPENAI_BASE_URL before appending the embedding path.
  • Documents the new setting in .env.example and README.

Notes

Test Plan

  • RED: npm test -- --run test/embedding-provider.test.ts failed because OPENAI_BASE_URL=http://localhost:11434/v1/ still produced /v1//v1/embeddings
  • GREEN: npm test -- --run test/embedding-provider.test.ts → 18 passed
  • npm run build
  • npm test → 91 files passed, 1008 tests passed

Summary by CodeRabbit

  • New Features

    • Added support for customizing the OpenAI embeddings endpoint path through the OPENAI_EMBEDDING_PATH environment variable, enabling better compatibility with proxy services using different routing paths.
  • Documentation

    • Updated README and configuration examples to document the new OPENAI_EMBEDDING_PATH environment variable and its usage.

Review Change Stack

@vercel
Copy link
Copy Markdown

vercel Bot commented May 17, 2026

@honor2030 is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

📝 Walkthrough

Walkthrough

This PR adds configurable OpenAI embeddings endpoint path support. It introduces OPENAI_EMBEDDING_PATH env var to override the default /v1/embeddings route, enabling compatibility with OpenAI-compatible proxies. Configuration is documented, implementation includes path normalization and URL building, and test coverage verifies the feature works end-to-end.

Changes

OpenAI Embedding Path Customization

Layer / File(s) Summary
Configuration and documentation
.env.example, README.md
Documents the new OPENAI_EMBEDDING_PATH environment variable for customizing the embeddings endpoint path when using proxies with non-standard routes.
Path normalization helper
src/providers/embedding/openai.ts
Introduces normalizeEmbeddingPath utility to standardize embedding path format (trim whitespace, ensure leading /, default to /v1/embeddings) and updates provider inline documentation.
Constructor initialization and URL building
src/providers/embedding/openai.ts
Constructor stores normalized embedding path from OPENAI_EMBEDDING_PATH env var and trims base URL trailing slashes; embedBatch uses configured path in request URL instead of hardcoded /v1/embeddings.
Test setup and coverage
test/embedding-provider.test.ts
Test suite clears OPENAI_EMBEDDING_PATH in setup to prevent env leakage; new test case verifies custom embedding path is correctly used in the fetch URL.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • rohitg00/agentmemory#186: Both PRs modify src/providers/embedding/openai.ts to dynamically construct the OpenAI embeddings request URL and update test/embedding-provider.test.ts to assert the fetch URL built from environment-derived values.

Poem

🐰 A path through the proxy we gently define,
No hardcoding walls, the embeddings align,
With OPENAI_EMBEDDING_PATH set just right,
Custom routes flow true, day or night! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: enabling custom OpenAI embedding paths.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/embedding-provider.test.ts (1)

98-114: ⚡ Quick win

Add a no-leading-slash path test to lock normalization behavior.

Please add one case with OPENAI_EMBEDDING_PATH="embeddings" and assert the request URL still resolves to /embeddings; this prevents regressions in normalizeEmbeddingPath.

🤖 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 `@test/embedding-provider.test.ts` around lines 98 - 114, Add a new test case
verifying that a path without a leading slash is normalized: create a test
similar to the existing "respects OPENAI_EMBEDDING_PATH..." case but set
process.env["OPENAI_EMBEDDING_PATH"] = "embeddings" (no leading slash),
instantiate OpenAIEmbeddingProvider("test-key"), spy/mock global fetch to return
a successful embedding response, call provider.embed("hello"), and assert fetch
was called with "http://localhost:11434/v1/embeddings" (use expect.any(Object)
for options); finally restore the fetch spy. This ensures normalizeEmbeddingPath
behavior for OpenAIEmbeddingProvider is locked.
🤖 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.

Nitpick comments:
In `@test/embedding-provider.test.ts`:
- Around line 98-114: Add a new test case verifying that a path without a
leading slash is normalized: create a test similar to the existing "respects
OPENAI_EMBEDDING_PATH..." case but set process.env["OPENAI_EMBEDDING_PATH"] =
"embeddings" (no leading slash), instantiate
OpenAIEmbeddingProvider("test-key"), spy/mock global fetch to return a
successful embedding response, call provider.embed("hello"), and assert fetch
was called with "http://localhost:11434/v1/embeddings" (use expect.any(Object)
for options); finally restore the fetch spy. This ensures normalizeEmbeddingPath
behavior for OpenAIEmbeddingProvider is locked.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: df1aaef4-6651-4fd4-861c-bb89380bcef4

📥 Commits

Reviewing files that changed from the base of the PR and between 9061da5 and df366a4.

📒 Files selected for processing (4)
  • .env.example
  • README.md
  • src/providers/embedding/openai.ts
  • test/embedding-provider.test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant