Skip to content

Conversation

@GunaPalanivel
Copy link
Contributor

@GunaPalanivel GunaPalanivel commented Jan 15, 2026

Related Issues

Proposed Changes

This PR fixes the Cohere Document Embedder and Ranker to not modify input Documents in place when setting embeddings or scores.

Changes Made

CohereDocumentEmbedder:

  • run() method: Use replace(doc, embedding=embeddings) instead of doc.embedding = embeddings
  • run_async() method: Use replace(doc, embedding=embeddings) instead of doc.embedding = embeddings

CohereRanker:

  • run() method: Use replace(doc, score=score) instead of doc.score = score

Note: CohereDocumentImageEmbedder already uses replace() correctly and didn't need changes.

Tests Added

  • test_run_does_not_modify_original_documents for document embedder
  • test_run_async_does_not_modify_original_documents for document embedder
  • test_run_does_not_modify_original_documents for ranker

How did you test it?

cd integrations/cohere
hatch run test:unit  # All 84 tests pass
hatch run fmt-check  # All checks pass

Notes for the reviewer

This follows the established pattern from:

This PR fixes the Cohere document embedder and ranker to not modify input
Documents in place when setting embeddings or scores.

Instead of mutating the original documents:
  doc.embedding = embeddings
  doc.score = score

We now create new document instances using dataclass replace:
  replace(doc, embedding=embeddings)
  replace(doc, score=score)

This follows the established pattern from haystack-ai/haystack#9693 and
aligns with other integrations (FastEmbed, Optimum, Nvidia, Bedrock).

Related to: deepset-ai#2174
Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

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

Looks good to me! @GunaPalanivel Thank you for your contribution. Great job! I'll release a new version of the integration.

@julian-risch julian-risch merged commit 8e48a7b into deepset-ai:main Jan 16, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants