Skip to content

feat!: add document_comparison_field to DocumentNDCGEvaluator#11636

Open
Kunal-Somani wants to merge 5 commits into
deepset-ai:mainfrom
Kunal-Somani:feat/document-ndcg-comparison-field
Open

feat!: add document_comparison_field to DocumentNDCGEvaluator#11636
Kunal-Somani wants to merge 5 commits into
deepset-ai:mainfrom
Kunal-Somani:feat/document-ndcg-comparison-field

Conversation

@Kunal-Somani

Copy link
Copy Markdown
Contributor

Related Issues

Proposed Changes

DocumentNDCGEvaluator was the only document evaluator in Haystack that hardcoded document matching by id. This caused a known limitation where ground truth and retrieved documents had to share the same auto-generated hash-based ID to be matched correctly, which is rarely the case in practice.

DocumentMAPEvaluator, DocumentMRREvaluator, and DocumentRecallEvaluator all already support a document_comparison_field parameter that lets users control how documents are matched. This PR brings DocumentNDCGEvaluator to the same level of consistency.

Changes:

  • Added document_comparison_field: str = "content" init parameter to DocumentNDCGEvaluator, defaulting to "content" for consistency with sibling evaluators
  • Added _get_comparison_value() method following the exact same pattern as DocumentMAPEvaluator and DocumentMRREvaluator
  • Added to_dict() for serialization support
  • Refactored calculate_dcg() to use the configurable comparison field instead of hardcoded doc.id lookup
  • Removed the TODO comment referencing Update Document.__eq__ to intelligently compare floats #8412
  • Added 10 new unit tests covering id, meta.<key>, nested meta, missing keys, unsupported fields, and serialization round-trips

How did you test it?

  • Ran the full evaluator test suite locally: PYTHONPATH="" hatch run test:unit test/components/evaluators/test_document_ndcg.py & 27 passed, 0 failures (17 existing + 10 new)
  • Ran type checks: hatch run test:types — no issues in 365 source files
  • Ran formatter: hatch run fmt — all checks passed
  • All pre-commit hooks passed at commit time

Notes for the reviewer

The default value is "content" (not "id") to match the behaviour of DocumentMAPEvaluator, DocumentMRREvaluator, and DocumentRecallEvaluator. This is a behaviour change from the previous hardcoded id matching, but the previous behaviour was incorrect for the common case where ground truth and retrieved documents are created independently and do not share IDs.

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes.
  • I have added unit tests and updated the docstrings.
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I have documented my code.
  • I have added a release note file, following the contributors guidelines.
  • I have run pre-commit hooks and fixed any issue.

@Kunal-Somani Kunal-Somani requested a review from a team as a code owner June 15, 2026 09:42
@Kunal-Somani Kunal-Somani requested review from bogdankostic and removed request for a team June 15, 2026 09:42
@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

@Kunal-Somani is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

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

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/components/evaluators
  document_ndcg.py
Project Total  

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

@bogdankostic bogdankostic 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.

Thanks for your PR @Kunal-Somani! I left a few in-line comments that should be addressed before merging.

Comment thread haystack/components/evaluators/document_ndcg.py Outdated
Comment thread haystack/components/evaluators/document_ndcg.py Outdated
Comment thread haystack/components/evaluators/document_ndcg.py
Comment thread haystack/components/evaluators/document_ndcg.py
@bogdankostic bogdankostic changed the title feat: add document_comparison_field to DocumentNDCGEvaluator feat!: add document_comparison_field to DocumentNDCGEvaluator Jun 19, 2026
@Kunal-Somani

Copy link
Copy Markdown
Contributor Author

Hey @bogdankostic
Thanks again for the detailed review. Let me know if any further changes are needed.

Comment thread haystack/components/evaluators/document_ndcg.py Outdated
Comment thread haystack/components/evaluators/document_ndcg.py Outdated
Comment thread haystack/components/evaluators/document_ndcg.py Outdated
Comment thread haystack/components/evaluators/document_ndcg.py Outdated
Comment thread haystack/components/evaluators/document_ndcg.py Outdated
Co-authored-by: bogdankostic <bogdankostic@web.de>
@Kunal-Somani

Copy link
Copy Markdown
Contributor Author

Thanks @bogdankostic

Pulled in your formatting fixes and everything's merged and all 28 tests still pass. Let me know if anything else is needed.

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.

2 participants