Skip to content

fix: ignore dimension for custom mock embeddings - #12980

Merged
anakin87 merged 2 commits into
deepset-ai:mainfrom
carey-bk:fix/mock-embedder-dimension-12978
Sep 28, 2026
Merged

anakin87 merged 2 commits into
deepset-ai:mainfrom
carey-bk:fix/mock-embedder-dimension-12978

Conversation

@carey-bk

Copy link
Copy Markdown
Contributor

Related Issues

Fixes #12978.

Proposed Changes

MockTextEmbedder and MockDocumentEmbedder rejected non-positive dimension
values even when a fixed embedding or embedding_fn made the parameter irrelevant.
Validate positive dimensions only in the default deterministic mode, matching the
documented contract. Preserve mutual exclusion and vector validation, clarify the
raises documentation, and add a release note.

The regression tests cover both components, zero and negative dimensions, actual
callback invocation, invalid vectors, and serialization round trips with importable
named functions. Supplied dimension values and the serialization format are unchanged.

How did you test it?

Latest upstream base checked: 8a5406eea71a0fc19e94c4b9a5cd96df2158a45a.

  • Re-ran the focused mock embedder suite after rebasing: 79 passed.
  • Re-ran hatch run fmt-check: lint and formatting passed.
  • All applicable pre-commit hooks passed when creating the commit.
  • The same regression tests previously produced 32 failed, 47 passed on the
    unmodified base and 79 passed with the fix.
  • Additional local validation of this unchanged patch: the embedder unit suite
    passed with 148 passed, 6 integration tests deselected, and mypy passed on
    the four changed Python files.

Focused test command (Python 3.12, Hatch environment with the required dependencies):

hatch run python -m pytest --cov=haystack -m 'not integration' \
  --disable-socket --allow-unix-socket \
  test/components/embedders/test_mock_text_embedder.py \
  test/components/embedders/test_mock_document_embedder.py

Tests use fixed inputs and offline mocks, with network sockets disabled. No model
weights were downloaded or model APIs called.

Notes for the reviewer

This patch was fully generated with Codex. The results above are local; remote CI
has not run. In the local environment, whole-tree mypy reported the same four
unused-ignore diagnostics on the base and patched code. The full dependency/CI
matrix remains unverified. Full-history reno lint was blocked by the shallow
clone; the new note passed YAML, reStructuredText, and project hook checks.

Checklist

  • Read the contributors guidelines and code of conduct.
  • Updated the related issue with new insights and changes.
  • Added regression coverage and updated the docstrings.
  • Used a conventional commit type for the PR title.
  • Documented the behavior and included a release note.
  • Ran the applicable pre-commit hooks and addressed their findings.

@carey-bk
carey-bk requested a review from a team as a code owner September 27, 2026 09:45
@carey-bk
carey-bk requested review from anakin87 and removed request for a team September 27, 2026 09:45
@vercel

vercel Bot commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

@carey-bk is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant

CLAassistant commented Sep 27, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot added topic:tests type:documentation Improvements on the docs labels Sep 27, 2026
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/components/embedders
  mock_document_embedder.py
  mock_text_embedder.py
Project Total  

This report was generated by python-coverage-comment-action

@anakin87 anakin87 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

I simplified the tests a bit

@anakin87
anakin87 enabled auto-merge (squash) September 28, 2026 09:15
@anakin87
anakin87 merged commit 0edc59c into deepset-ai:main Sep 28, 2026
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MockTextEmbedder and MockDocumentEmbedder reject dimension<=0 even when ignored

3 participants