Skip to content

add bge-m3 embedding dimension alias#767

Open
TurboTheTurtle wants to merge 2 commits into
CortexReach:masterfrom
TurboTheTurtle:fix-bge-m3-model-alias
Open

add bge-m3 embedding dimension alias#767
TurboTheTurtle wants to merge 2 commits into
CortexReach:masterfrom
TurboTheTurtle:fix-bge-m3-model-alias

Conversation

@TurboTheTurtle
Copy link
Copy Markdown
Contributor

Summary

Allow users to configure Huawei/BAAI BGE-M3 with the plain bge-m3 model name without also having to set embedding.dimensions manually.

  • Add bge-m3 as a 1024-dimension alias alongside the existing BAAI/bge-m3 entry.
  • Extend the embedder regression test so both common model identifiers resolve to 1024 dimensions.

Fixes #339

Validation

  • node test/embedder-error-hints.test.mjs
  • git diff --check

@rwmjhb
Copy link
Copy Markdown
Collaborator

rwmjhb commented May 9, 2026

Please rebase this branch on current master and rerun CI before merge.

The bge-m3 alias change is small and useful, but this branch is behind current master and currently reports packaging/core failures. A fresh rebase should keep this as a minimal embedder/test-only diff.

@TurboTheTurtle TurboTheTurtle force-pushed the fix-bge-m3-model-alias branch from d46806d to 1face9d Compare May 12, 2026 07:54
Copy link
Copy Markdown
Collaborator

@rwmjhb rwmjhb left a comment

Choose a reason for hiding this comment

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

PR #767 Review: add bge-m3 embedding dimension alias

Verdict: APPROVE | 6 rounds completed | Value: 52% | Size: SMALL | Author: TurboTheTurtle

Value Assessment

Problem: Users configuring Huawei/BAAI BGE-M3 with the plain bge-m3 model name hit an unsupported-model startup error unless they set embedding.dimensions; setting dimensions then causes the provider to reject the request. The PR aims to make bge-m3 resolve to the known 1024-dimension default without requiring a dimensions override.

Dimension Assessment
Value Score 52%
Value Verdict review
Issue Linked true
Project Aligned true
Duplicate false
AI Slop Score 0/6
User Impact medium
Urgency medium

Scope Drift: 2 flag(s)

  • index.ts export of isLayer1CircuitOpen is not justified by the BGE-M3 alias problem statement
  • test/issue606_sdk-migration.test.mjs jiti loader changes are CI/test-harness related and not mentioned in the PR summary

Open Questions:

  • Is bge-m3 already present on current master after this stale branch diverged?
  • Is exporting isLayer1CircuitOpen intended as public API, or should the SDK migration test avoid requiring a new export?

Summary

Users configuring Huawei/BAAI BGE-M3 with the plain bge-m3 model name hit an unsupported-model startup error unless they set embedding.dimensions; setting dimensions then causes the provider to reject the request. The PR aims to make bge-m3 resolve to the known 1024-dimension default without requiring a dimensions override.

Evaluation Signals

Signal Value
Blockers 0
Warnings 0
PR Size SMALL
Verdict Floor approve
Risk Level high
Value Model codex
Primary Model codex
Adversarial Model claude

Recommended Action

Ready to merge.


Reviewed at 2026-05-12T15:28:01Z | 6 rounds | Value: codex | Primary: codex | Adversarial: claude

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.

Unsupported embedding model: bge-m3. Either add it to EMBEDDING_DIMENSIONS or set embedding.dimensions in config

2 participants