Skip to content

feat(embedding): declare a model's width without sending dimensions - #1857

Open
NullSense wants to merge 1 commit into
repowise-dev:mainfrom
NullSense:feat/declared-only-embedding-dims
Open

feat(embedding): declare a model's width without sending dimensions#1857
NullSense wants to merge 1 commit into
repowise-dev:mainfrom
NullSense:feat/declared-only-embedding-dims

Conversation

@NullSense

Copy link
Copy Markdown

Summary

  • OpenAIEmbedder can only override an embedding model's width by also sending that width to the API as the dimensions parameter. That's correct for a Matryoshka model, but it breaks a model of non-default width that isn't Matryoshka-capable: the API rejects the request outright (e.g. nvidia/Nemotron-3-Embed-1B, 2048-wide, served OpenAI-compatible, errors with "does not support Matryoshka embeddings; dimensions must be unset").
  • Adds declared_dimensions / REPOWISE_EMBEDDING_DECLARED_DIMS, a second override that sets the declared width without ever sending it. Precedence: dimensions= > REPOWISE_EMBEDDING_DIMS > declared_dimensions= > REPOWISE_EMBEDDING_DECLARED_DIMS > the _DIMS table > 1536. Only the first two are ever sent on the wire.
  • The width-mismatch error now names whichever of the three sources actually chose the number, instead of always pointing at _DIMS or REPOWISE_EMBEDDING_DIMS.

Related Issues

None.

Test Plan

  • uv run pytest tests/unit/test_persistence/test_openai_embedder.py -q — 52 passed
  • uv run pytest tests/unit/test_persistence/ tests/unit/test_providers/ tests/unit/server/mcp/test_embedder_resolution.py tests/unit/cli/test_embedder_key_resolution.py -q — 416 passed
  • uv run ruff check . (the actual CI lint gate; ruff format --check isn't run in CI, and this PR's files are clean under it anyway)
  • uv run repowise risk main..HEAD:
    Change risk for main..HEAD: touches files that have broken before · 52nd percentile of this repo's fix-bearing files
      feat(embedding): declare a model's width without sending dimensions
      +212 / -36 lines · 2 files · 2 dirs · 2 subsystems · entropy 0.98 · author exp 0
    
    File                                                              Lines   Prior fixes
    packages/core/src/repowise/core/providers/embedding/openai.py       142           1.0
    tests/unit/test_persistence/test_openai_embedder.py                 106           1.0
    
    Diff shape: Typical · 48th percentile of recent commits by size and spread
      About as risky as a typical commit in this repo.
    
    (impacted-tests / health --file need a completed repowise init against this clone; that indexing run did not finish in time and is omitted rather than faked.)

Why this shape, not a _DIMS table entry

The straightforward fix is hardcoding "nemotron-embed-1b": 2048 in _DIMS. That covers exactly one deployment; every other non-Matryoshka model of non-default width hits the identical wall. The existing dimensions= design already treats width as something the operator declares for whatever model they're running, not something the code enumerates by name — declared_dimensions extends that same idea to the one case it didn't cover.

Checklist

  • My code follows the project's code style
  • I have added tests for new functionality
  • All existing tests still pass
  • I have updated documentation if needed

Written with AI assistance; measurements and tests were run and verified locally.

@repowise-bot

repowise-bot Bot commented Aug 22, 2026

Copy link
Copy Markdown

✅ Health of changed files: 6.5 → 6.9 (+0.4)
⚠️ Change risk: moderate, riskier than 48% of this repo's commits.

📋 At a glance
1 file changed health · 2 hotspots touched · 1 new finding introduced · 2 files with recent fix history.

Files & modules (2)
  • packages (1 file)
    • .../embedding/openai.py
  • tests (1 file)
    • .../test_persistence/test_openai_embedder.py

✅ Health gate: passed

📌 Before you merge

  • Run .../mcp/test_embedder_resolution.py, .../test_persistence/test_embedding_timeout_resolution.py, .../test_providers/test_deepseek_provider.py: they import the changed files
🔎 More signals (2)

🗺️ Change map

flowchart LR
  subgraph PR ["Changed in this PR (1 with dependents)"]
    f_packages_core_src_repowise_core_providers_embedding_openai_py[".../embedding/openai.py 🔥"]:::changed
  end
  f_packages_core_src_repowise_core_providers_embedding_edenai_py[".../embedding/edenai.py"]
  f_packages_core_src_repowise_core_providers_embedding_openai_py --> f_packages_core_src_repowise_core_providers_embedding_edenai_py
  f_packages_core_src_repowise_core_providers_embedding_openrouter_py[".../embedding/openrouter.py"]
  f_packages_core_src_repowise_core_providers_embedding_openai_py --> f_packages_core_src_repowise_core_providers_embedding_openrouter_py
  f_packages_core_src_repowise_core_providers_embedding_registry_py[".../embedding/registry.py"]
  f_packages_core_src_repowise_core_providers_embedding_openai_py --> f_packages_core_src_repowise_core_providers_embedding_registry_py
  f_packages_core_src_repowise_core_providers_llm_deepseek_py[".../llm/deepseek.py"]
  f_packages_core_src_repowise_core_providers_embedding_openai_py --> f_packages_core_src_repowise_core_providers_llm_deepseek_py
  more(["+6 more dependents"])
  PR --> more
  t_tests_unit_server_mcp_test_embedder_resolution_py(["✅ .../mcp/test_embedder_resolution.py"]):::guard
  t_tests_unit_server_mcp_test_embedder_resolution_py -.-> f_packages_core_src_repowise_core_providers_embedding_openai_py
  classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
  classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
  classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Loading

Solid arrows: code that imports the changed files (10 direct dependents, from the last indexed snapshot). Dashed: history/tests.

🔥 Hotspots touched (2)

  • .../test_persistence/test_openai_embedder.py: 3 commits/90d, 0 dependents · primary owner: Sergei Iakhnitskii (42%)
  • .../embedding/openai.py: 4 commits/90d, 21 dependents · primary owner: Raghav Chamadiya (53%)

👀 Suggested reviewers @RaghavChamadiya


📊 See the full report for this PR
Your repo map with this PR's blast radius lit up, every caller of the contracts it changes, and health before and after. No sign-in. · ⭐ Star Repowise · 📥 Install bot · Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot · Updated 2026-08-22 15:47 UTC

@Ayush7614 Ayush7614 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified: pytest tests/unit/test_persistence/test_openai_embedder.py -q 52 passed. Correctly adds declared_dimensions/REPOWISE_EMBEDDING_DECLARED_DIMS as declare-only width (never sent as dimensions param) for non-Matryoshka models like Nemotron-3-Embed-1B that 400 on dimensions. Precedence dimensions > REPOWISE_EMBEDDING_DIMS > declared_dimensions > REPOWISE_EMBEDDING_DECLARED_DIMS > _DIMS > 1536 matches spec, error messages now name the actual source, and the strict-endpoint regression test proves the param is never sent. LGTM.

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.

2 participants