Skip to content

perf(deps): route Cohere & Mistral through OpenAI-compat (-1.8 MB bundle)#2402

Merged
logancyang merged 1 commit into
masterfrom
feat/replace-cohere-mistral-with-openai-compat
May 12, 2026
Merged

perf(deps): route Cohere & Mistral through OpenAI-compat (-1.8 MB bundle)#2402
logancyang merged 1 commit into
masterfrom
feat/replace-cohere-mistral-with-openai-compat

Conversation

@logancyang
Copy link
Copy Markdown
Owner

@logancyang logancyang commented May 12, 2026

Summary

Replaces ChatCohere, ChatMistralAI, and CohereEmbeddings with ChatOpenAI / OpenAIEmbeddings pointed at each provider's OpenAI-compatible endpoint:

  • Cohere: https://api.cohere.ai/compatibility/v1
  • Mistral: https://api.mistral.ai/v1 (already OpenAI-shaped)

Drops @langchain/cohere, @langchain/mistralai, cohere-ai, and @mistralai/mistralai from package.json. This also eliminates the @aws-sdk/* + @smithy/* chain that cohere-ai was transitively pulling in for Bedrock auth (a path this plugin doesn't use).

Bundle impact (production build, minified)

Before After
main.js 5.21 MB 3.39 MB

Saves ~1.8 MB (~35%).

Test plan

  • npm run lint clean
  • npm test — 1962 / 1962 pass
  • npx tsc -noEmit clean
  • Production bundle drops from 5.21 MB → 3.39 MB
  • Manual: Cohere chat — add a Cohere model (e.g. command-r), send a message, confirm response streams correctly
  • Manual: Cohere embeddings — set Cohere as embedding provider, build a vault index, confirm semantic search returns results
  • Manual: Mistral chat — add a Mistral model (e.g. mistral-large-latest), send a message, confirm streaming + tool use work
  • Manual: Cohere tool use — try agent mode with a Cohere model that supports tools (command-a-*); confirm tool calls dispatch correctly
  • Manual: existing settings — open the plugin with prior Cohere/Mistral configs and confirm models still load (no migration needed; provider keys unchanged)

Notes / Risks

  • The Cohere compatibility endpoint maps to OpenAI's chat-completions shape but may differ from native Cohere in subtle ways (e.g. it doesn't expose Cohere's input_type parameter for embeddings — Cohere selects the right type based on the OpenAI-style request). If retrieval quality regresses on existing Cohere indexes, that's the likely cause.
  • listModelURL for Cohere still points at native /v1/models since that's known to work; the compat endpoint may or may not expose models OpenAI-style. Runtime chat/embedding calls go through the compat URL.

One-time migration note for release notes

The previous ChatCohere wrapper silently ignored any baseUrl saved on a custom Cohere model. After this PR, Cohere honors baseUrl consistently with every other provider in this file. If a user had pasted https://api.cohere.com into the baseUrl field (e.g. by copying the placeholder), Cohere requests will now hit https://api.cohere.com/chat/completions and fail with a 404. The fix is to clear the baseUrl field for that Cohere model so it falls back to the compatibility endpoint. Worth a line in the release notes.

🤖 Generated with Claude Code

…e by 1.8MB

Replace ChatCohere / ChatMistralAI / CohereEmbeddings with ChatOpenAI /
OpenAIEmbeddings pointed at each provider's OpenAI-compatible endpoint:

- Cohere: https://api.cohere.ai/compatibility/v1
- Mistral: https://api.mistral.ai/v1 (already OpenAI-shaped)

Dropping @langchain/cohere, @langchain/mistralai, cohere-ai, and
@mistralai/mistralai also eliminates the @aws-sdk/* and @smithy/* chain
that cohere-ai dragged in for Bedrock auth (a path this plugin doesn't use).

Production bundle: 5.21MB -> 3.39MB.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 42e13210f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

apiKey: await getDecryptedKey(customModel.apiKey || settings.cohereApiKey),
model: modelName,
configuration: {
baseURL: customModel.baseUrl || ProviderInfo[ChatModelProviders.COHEREAI].host,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Migrate legacy Cohere base URLs before using ChatOpenAI

This now forwards any saved customModel.baseUrl directly into ChatOpenAI, but existing Cohere configs may still have the old native host (https://api.cohere.com) from pre-compat setups. Those values are no longer valid for OpenAI-style routes, so requests are sent to /chat/completions on a non-compat endpoint and fail for affected users. Previously this field was effectively ignored for Cohere, so this is a regression for existing saved model configs unless you normalize or override legacy URLs to the compatibility base.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This is intentional. The previous ChatCohere wrapper ignored customModel.baseUrl entirely, so anyone with a stale https://api.cohere.com saved in that field was unaffected only because the value was being thrown away. Now that we route Cohere through ChatOpenAI, honoring baseUrl is the consistent choice across all other providers in this file (DeepSeek, SiliconFlow, OPENAI_FORMAT, etc.) and enables legitimate use cases like Cohere-compatible proxies.

The trade-off:

  • Stale api.cohere.com users (only those who had typed it into the field, since the previous code ignored it) will see a 404 and need to clear the baseUrl field. Loud, recoverable, easily documented.
  • Pinning to the compat URL forever would silently override user intent and lock out proxy users.

ProviderInfo[COHEREAI].host is updated in this PR to the compat URL, so the placeholder shown in ModelAddDialog now points users at the correct URL going forward. Will call out the migration step in release notes.

timeout: 10000,
batchSize: getSettings().embeddingBatchSize,
configuration: {
baseURL: customModel.baseUrl || ProviderInfo[EmbeddingModelProviders.COHEREAI].host,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid honoring stale Cohere baseUrl in embedding config

The embedding path now also uses customModel.baseUrl directly with OpenAIEmbeddings. If a user previously saved a native Cohere URL (for example https://api.cohere.com), embedding calls are routed to an OpenAI-style /embeddings path that does not match the old API shape, causing index/build failures for those migrated configs. This changed behavior from the previous Cohere embedding client path and needs URL migration/normalization for backwards compatibility.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This is intentional. The previous ChatCohere wrapper ignored customModel.baseUrl entirely, so anyone with a stale https://api.cohere.com saved in that field was unaffected only because the value was being thrown away. Now that we route Cohere through ChatOpenAI, honoring baseUrl is the consistent choice across all other providers in this file (DeepSeek, SiliconFlow, OPENAI_FORMAT, etc.) and enables legitimate use cases like Cohere-compatible proxies.

The trade-off:

  • Stale api.cohere.com users (only those who had typed it into the field, since the previous code ignored it) will see a 404 and need to clear the baseUrl field. Loud, recoverable, easily documented.
  • Pinning to the compat URL forever would silently override user intent and lock out proxy users.

ProviderInfo[COHEREAI].host is updated in this PR to the compat URL, so the placeholder shown in ModelAddDialog now points users at the correct URL going forward. Will call out the migration step in release notes.

@logancyang logancyang merged commit b7ad366 into master May 12, 2026
2 checks passed
@logancyang logancyang deleted the feat/replace-cohere-mistral-with-openai-compat branch May 12, 2026 21:54
logancyang pushed a commit that referenced this pull request May 13, 2026
Third of nine workspaces splitting #2397. Surfaces previously
swallowed promise rejections via .catch(logError) or `void` prefix.
No logic changes - only error-handling explicitness.

- ~67 floating promises now use void / .catch(logError)
- Promise-returning callbacks where void was expected are wrapped
- ConfirmModal accepts onConfirm/onCancel returning void | Promise<void>;
  rejections logged via logError
- Custom command runners wrap `result instanceof Promise` paths
- ChatUIState.replaceMessages is now async to match its delegate
- ChainManager initialize / createChainWithNewModel are explicitly
  voided with .catch(logError) at call sites
- dbOperations subscribeToSettingsChange uses void IIFE with try/catch

Behavior change: errors previously hidden by floating promises now
log via @/logger. No code paths changed.

W0, W1, and #2402 already merged.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
logancyang pushed a commit that referenced this pull request May 13, 2026
…2406)

* fix(popout): document ownership and selection listener fixes (W4/9)

Fifth of nine workspaces splitting #2397. Real popout-window
correctness fixes — semantic choices about which window owns a DOM
operation, plus selection-listener captured-document fixes.

- document → activeDocument where a generic active-window lookup is
  correct (SourcesModal, ChatViewLayout, ItemView/Notice mocks,
  TabContext, ChatInput keydown listener, CopilotView keyboard
  observer, TypeaheadMenuPortal, AddImageModal, and ?? document
  fallbacks across draggable-modal, menu-command-modal,
  CustomCommandChatModal, use-draggable, use-resizable, and
  QuickAskOverlay).
- document → root.ownerDocument in ChatSingleMessage's
  linkInlineCitations so DOM nodes end up in the citation's window;
  same pattern in the processMessage streaming block via
  contentRef.current.ownerDocument.
- New getEditorDocument(editor) helper in BasePillNode. createDOM and
  exportDOM now accept the LexicalEditor; pills create DOM in the
  editor's window (correct popout behavior). All 6 pill subclasses
  updated to match the new signature.
- main.ts: selectionListenerDocument captured at listener registration;
  remove uses the same document (fixes leak when activeDocument has
  drifted before unload).
- chatSelectionHighlightController.ts: getActiveViewOfType(MarkdownView)
  replaces the brittle activeLeaf type comparison.
- Test setup: (window as any).activeDocument = window.document in
  ChatSingleMessage.test; window.document.createElement in
  AgentPrompt.test Notice mock.

Behavior fix: chat in a popout window now creates DOM nodes in the
popout's document, not the focused window's document.

W0, W1, and #2402 already merged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(popout): prefer element.doc/.win over ownerDocument and activeDocument

Switch popout-window-sensitive call sites to Obsidian's `.doc` / `.win`
augmentations so DOM ops, listeners, portals, and positioning follow the
element's actual owner window. Recreate Lexical root on view migration to
rebind input handling after a leaf moves between windows. Document the
decision order in AGENTS.md and polyfill `Node.doc` / `Node.win` in
jest.setup.js so tests still run under jsdom.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* improve image upload implementation

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
zeroliu added a commit that referenced this pull request May 13, 2026
…e cleanup (W6/9)

Seventh of nine workspaces splitting #2397. Picks up provider-file
changes W1 deferred — primarily LangChain public-API renames that
aren't pure type changes.

- src/LLMProviders/BedrockChatModel.ts: _getType() -> getType()
  (non-deprecated method, same behavior)
- src/LLMProviders/BedrockChatModel.test.ts: matching mock-message
  rename across all streaming-decode tests
- src/LLMProviders/chainRunner/CopilotPlusChainRunner.ts: stringify
  non-string response.content via JSON.stringify instead of String()
  (avoids [object Object] when content is structured)

No semantic message-handling changes. All behavior preserved.

W0, W1, and #2402 already merged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zeroliu added a commit that referenced this pull request May 13, 2026
…e cleanup (W6/9)

Seventh of nine workspaces splitting #2397. Picks up provider-file
changes W1 deferred — primarily LangChain public-API renames that
aren't pure type changes.

- src/LLMProviders/BedrockChatModel.ts: _getType() -> getType()
  (non-deprecated method, same behavior)
- src/LLMProviders/BedrockChatModel.test.ts: matching mock-message
  rename across all streaming-decode tests
- src/LLMProviders/chainRunner/CopilotPlusChainRunner.ts: stringify
  non-string response.content via JSON.stringify instead of String()
  (avoids [object Object] when content is structured)

No semantic message-handling changes. All behavior preserved.

W0, W1, and #2402 already merged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zeroliu added a commit that referenced this pull request May 13, 2026
…W7/9)

Eighth of nine workspaces splitting #2397. Intentional behavior change:
file deletions now honor the user's Settings -> Files and links ->
Deleted files preference (system trash / .trash / permanent), instead
of unconditionally calling vault.delete(file, true).

- src/utils/vaultAdapterUtils.ts: add trashFile(app, file) helper that
  casts past the missing app.fileManager.trashFile typedef.
  createSyntheticTFile now uses Object.create(TFile.prototype) so
  instanceof TFile returns true.
- src/projects/ProjectFileManager.ts: constructor takes App instead
  of Vault; all vault.delete(file, true) -> trashFile(app, file).
  Project create rollback now trashes partial files (was permanent-
  delete before).
- src/projects/projectRegister.ts: constructor takes App.
- src/projects/projectMigration.ts: rollbackCreatedFile(app, ...),
  ensureProjectsMigratedIfNeeded(app), all delete sites use trashFile.
- src/main.ts: deleteChatHistory uses trashFile; ProjectRegister
  constructed with this.app.
- src/system-prompts/systemPromptManager.ts: prompt delete via trashFile.
- src/commands/customCommandManager.ts: command delete via trashFile.
- src/LLMProviders/projectManager.ts, src/components/Chat.tsx,
  src/components/chat-components/ProjectList.tsx: pass app (not vault)
  to ProjectFileManager.getInstance.
- __mocks__/obsidian.js: app.fileManager.trashFile jest mock.
- Tests updated to expect trashFile + App constructor arg.

W0, W1, and #2402 already merged. Behavior change requires manual
verification (test plan section 1 + 6).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zeroliu added a commit that referenced this pull request May 13, 2026
…e cleanup (W6/9) (#2404)

Seventh of nine workspaces splitting #2397. Picks up provider-file
changes W1 deferred — primarily LangChain public-API renames that
aren't pure type changes.

- src/LLMProviders/BedrockChatModel.ts: _getType() -> getType()
  (non-deprecated method, same behavior)
- src/LLMProviders/BedrockChatModel.test.ts: matching mock-message
  rename across all streaming-decode tests
- src/LLMProviders/chainRunner/CopilotPlusChainRunner.ts: stringify
  non-string response.content via JSON.stringify instead of String()
  (avoids [object Object] when content is structured)

No semantic message-handling changes. All behavior preserved.

W0, W1, and #2402 already merged.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
zeroliu added a commit that referenced this pull request May 13, 2026
…W7/9)

Eighth of nine workspaces splitting #2397. Intentional behavior change:
file deletions now honor the user's Settings -> Files and links ->
Deleted files preference (system trash / .trash / permanent), instead
of unconditionally calling vault.delete(file, true).

- src/utils/vaultAdapterUtils.ts: add trashFile(app, file) helper that
  casts past the missing app.fileManager.trashFile typedef.
  createSyntheticTFile now uses Object.create(TFile.prototype) so
  instanceof TFile returns true.
- src/projects/ProjectFileManager.ts: constructor takes App instead
  of Vault; all vault.delete(file, true) -> trashFile(app, file).
  Project create rollback now trashes partial files (was permanent-
  delete before).
- src/projects/projectRegister.ts: constructor takes App.
- src/projects/projectMigration.ts: rollbackCreatedFile(app, ...),
  ensureProjectsMigratedIfNeeded(app), all delete sites use trashFile.
- src/main.ts: deleteChatHistory uses trashFile; ProjectRegister
  constructed with this.app.
- src/system-prompts/systemPromptManager.ts: prompt delete via trashFile.
- src/commands/customCommandManager.ts: command delete via trashFile.
- src/LLMProviders/projectManager.ts, src/components/Chat.tsx,
  src/components/chat-components/ProjectList.tsx: pass app (not vault)
  to ProjectFileManager.getInstance.
- __mocks__/obsidian.js: app.fileManager.trashFile jest mock.
- Tests updated to expect trashFile + App constructor arg.

W0, W1, and #2402 already merged. Behavior change requires manual
verification (test plan section 1 + 6).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zeroliu added a commit that referenced this pull request May 13, 2026
…W7/9) (#2405)

* fix(vault): respect user trash preference via FileManager.trashFile (W7/9)

Eighth of nine workspaces splitting #2397. Intentional behavior change:
file deletions now honor the user's Settings -> Files and links ->
Deleted files preference (system trash / .trash / permanent), instead
of unconditionally calling vault.delete(file, true).

- src/utils/vaultAdapterUtils.ts: add trashFile(app, file) helper that
  casts past the missing app.fileManager.trashFile typedef.
  createSyntheticTFile now uses Object.create(TFile.prototype) so
  instanceof TFile returns true.
- src/projects/ProjectFileManager.ts: constructor takes App instead
  of Vault; all vault.delete(file, true) -> trashFile(app, file).
  Project create rollback now trashes partial files (was permanent-
  delete before).
- src/projects/projectRegister.ts: constructor takes App.
- src/projects/projectMigration.ts: rollbackCreatedFile(app, ...),
  ensureProjectsMigratedIfNeeded(app), all delete sites use trashFile.
- src/main.ts: deleteChatHistory uses trashFile; ProjectRegister
  constructed with this.app.
- src/system-prompts/systemPromptManager.ts: prompt delete via trashFile.
- src/commands/customCommandManager.ts: command delete via trashFile.
- src/LLMProviders/projectManager.ts, src/components/Chat.tsx,
  src/components/chat-components/ProjectList.tsx: pass app (not vault)
  to ProjectFileManager.getInstance.
- __mocks__/obsidian.js: app.fileManager.trashFile jest mock.
- Tests updated to expect trashFile + App constructor arg.

W0, W1, and #2402 already merged. Behavior change requires manual
verification (test plan section 1 + 6).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* chore(manifest): bump minAppVersion to 1.4.0

FileManager.trashFile (used by the vault deletion helper introduced in
W7/9) requires Obsidian 1.4+. Bumping minAppVersion ensures the plugin
store only offers this and future releases to compatible app versions;
users on older Obsidian builds stay on their current Copilot version
instead of receiving an update that would crash on delete.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@logancyang logancyang mentioned this pull request May 13, 2026
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