Skip to content

fix(dedup): preserve distinct texts shorter than the n-gram size - #526

Merged
Lavender1 merged 1 commit into
OpenDCAI:mainfrom
zoedsy:fix/minhash-short-texts
Sep 10, 2026
Merged

fix(dedup): preserve distinct texts shorter than the n-gram size#526
Lavender1 merged 1 commit into
OpenDCAI:mainfrom
zoedsy:fix/minhash-short-texts

Conversation

@zoedsy

@zoedsy zoedsy commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

With the default ngram=5, ["cat", "dog", "cat"] currently becomes ["cat"]: no n-grams are generated for either short text, so both receive the same empty MinHash signature.

Use each non-empty text shorter than the n-gram size as a single shingle. Distinct short texts now survive while actual duplicates are removed. Empty texts retain their existing behavior; normal n-gram generation, character mode, and the per-run index lifetime are unchanged.

Validation: 8 CPU regression cases pass, covering English and Chinese short texts, a custom n-gram size, empty strings, actual duplicates, boundary and longer texts, character mode, and repeated runs. The unmodified code fails 5 cases and passes 3; all 8 pass with the fix. Tested with datasketch 2.0.0.

Copilot AI 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.

🟢 Approval recommended

The fix is narrowly scoped, preserves existing behavior for other cases, and is backed by targeted regression tests that cover the reported failure modes.

Pull request overview

This PR fixes an edge case in MinHashDeduplicateFilter where non-empty texts shorter than the configured n-gram size produced an empty MinHash signature and were incorrectly deduplicated together. The change ensures short-but-distinct texts remain distinct while keeping empty-string and existing n-gram/character-mode behavior unchanged.

Changes:

  • Update MinHash shingling so 0 < len(text) < ngram uses the whole text as a single shingle.
  • Add CPU-only regression tests covering short English/Chinese texts, custom n-gram sizes, empty strings, boundary/longer texts, character mode, and repeated operator runs.
File summaries
File Description
dataflow/operators/general_text/filter/minhash_deduplicate_filter.py Adjusts MinHash signature construction for short texts to avoid empty signatures and incorrect deduplication.
test/cpu_only/test_minhash_short_texts.py Adds regression tests validating correct behavior for short texts and ensuring existing behavior remains stable.
Review details
  • Files reviewed: 2/2 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.

@Lavender1

Copy link
Copy Markdown
Contributor

LGTM, Sincerely, thanks!

@Lavender1
Lavender1 merged commit c8c0994 into OpenDCAI:main Sep 10, 2026
6 checks passed
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.

3 participants