Repository navigation
Conversation
SparseBinaryClassificationEvaluator, SparseEmbeddingSimilarityEvaluator and SparseTripletEvaluator each override _append_csv_headers to append their active_dims / sparsity_ratio columns. Their dense superclasses call that hook twice whenever similarity_fn_names is left unset, which is the default: once from __init__ with an empty list, and again from __call__ once model.similarity_fn_name has filled it in. The metric columns are only added on the second call, but the sparse columns were appended unconditionally, so they landed in csv_headers both times. The row does not grow to match. The metric values come from the scores dict and the sparsity values are appended once by append_to_last_row, so the header ended up two columns wider than every row (six, for the triplet evaluator) and every value was filed under the wrong column name. This is the same bug as #3512, fixed for SparseInformationRetrievalEvaluator in #3456 but not for these three. Apply the guard that SparseInformationRetrievalEvaluator._append_csv_headers and SparseNanoBEIREvaluator._append_csv_headers already use: only append the sparse columns when the superclass is also adding metric columns.
|
Hello! Thanks for the fix! I've moved the tests into
|
There was a problem hiding this comment.
🟢 Approval recommended
The fix is narrowly scoped, matches established patterns in similar evaluators, and is covered by targeted regression tests that validate CSV header/row alignment.
Pull request overview
This PR fixes a CSV serialization bug in sparse evaluators where sparse-specific columns were being appended to csv_headers twice (when similarity_fn_names is left unset), causing header/row misalignment and duplicated header columns. The change aligns these evaluators with the existing guard pattern already used in other sparse evaluators.
Changes:
- Guard sparse-specific header appends in
SparseBinaryClassificationEvaluator,SparseEmbeddingSimilarityEvaluator, andSparseTripletEvaluatorso they only run whensimilarity_fn_namesis non-empty (i.e., when metric columns are also being added). - Add regression tests that run each evaluator twice and assert the written CSV header matches the expected columns and aligns exactly with written row values.
- Add
tests/sparse_encoder/evaluation/__init__.py(for package/annotation consistency in the new test directory).
File summaries
| File | Description |
|---|---|
| sentence_transformers/sparse_encoder/evaluation/sparse_binary_classification.py | Only append active_dims / sparsity_ratio headers when metric headers are appended. |
| sentence_transformers/sparse_encoder/evaluation/sparse_embedding_similarity.py | Only append active_dims / sparsity_ratio headers when metric headers are appended. |
| sentence_transformers/sparse_encoder/evaluation/sparse_triplet.py | Only append per-role sparsity headers when metric headers are appended. |
| tests/sparse_encoder/evaluation/test_binary_classification_evaluator.py | Regression test: header/row alignment for sparse binary classification evaluator CSV. |
| tests/sparse_encoder/evaluation/test_embedding_similarity_evaluator.py | Regression test: header/row alignment for sparse embedding similarity evaluator CSV. |
| tests/sparse_encoder/evaluation/test_triplet_evaluator.py | Regression test: header/row alignment for sparse triplet evaluator CSV. |
| tests/sparse_encoder/evaluation/init.py | Initializes the new test package (future annotations). |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
SparseBinaryClassificationEvaluator,SparseEmbeddingSimilarityEvaluatorandSparseTripletEvaluatorwrite a CSV whose header has more columns than its rows, so every value ends up under the wrong column name. It is the same bug as #3512, which you fixed forSparseInformationRetrievalEvaluatorin #3456 but which is still there in these three.Each of them overrides
_append_csv_headersto append itsactive_dims/sparsity_ratiocolumns (sparse_binary_classification.py:136,sparse_embedding_similarity.py:114,sparse_triplet.py:129). The dense superclass calls that hook twice wheneversimilarity_fn_namesis left unset, which is the default: once from__init__with an empty list, and again from__call__oncemodel.similarity_fn_namehas filled it in (binary_classification.py:121and:180,embedding_similarity.py:132and:187,triplet.py:157and:196). The metric columns are only added on the second call, but the sparse columns were appended unconditionally, so they landed incsv_headersboth times.The row does not grow to match, because the metric values come from the scores dict and the sparsity values are appended once by
append_to_last_row.SparseTripletEvaluatoronmainwrites a 15 column header over a 9 column row:The fix is the guard
SparseInformationRetrievalEvaluator._append_csv_headersandSparseNanoBEIREvaluator._append_csv_headersalready use: append the sparse columns only when the superclass is adding metric columns too. Passingsimilarity_fn_namesexplicitly was already fine and stays fine, since then only the__init__call adds anything.The new test builds every
SparseEncoderevaluator that writes a CSV and checks the header has no repeated column and exactly as many cells as the row.I also checked the CSVs the other sparse evaluators write, before and after:
SparseInformationRetrievalEvaluator,SparseNanoBEIREvaluator,SparseRerankingEvaluator,SparseTranslationEvaluatorandSparseMSEEvaluatorare unaffected, and repeated calls appending to an existing file stay aligned.