From 7ad68f28ec2b0e8464b3e55cedf270812e61a093 Mon Sep 17 00:00:00 2001 From: 0xDevNinja Date: Thu, 16 Jul 2026 18:44:14 +0530 Subject: [PATCH] fix(corpus): normalize diversify() position term so similarity can compete The greedy score mixed incommensurate scales: cosine similarity bounded to [-1, 1] against an absolute list index that grows with the pool. For a candidate j positions later to be preferred at the default diversity=0.5, its similarity advantage had to exceed j -- impossible for the non-negative cosines real footage embeddings produce. diversify() therefore returned the input order verbatim, placing exact-duplicate clips in adjacent edit slots, the one thing its docstring promises to prevent. The threshold where the knob started working also depended on pool size (0.66 at 4 candidates, 0.95 at 11). Normalize the position term to [0, 1] so both terms share a scale. The documented endpoints hold exactly as before: diversity=0 returns input order, diversity=1 picks the most mutually dissimilar. Enumerating the position also drops the O(n^2) remaining.index() lookup per candidate. Closes #392 --- lib/corpus.py | 11 +- tests/lib/test_corpus_diversify_scale.py | 126 +++++++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 tests/lib/test_corpus_diversify_scale.py diff --git a/lib/corpus.py b/lib/corpus.py index 4aefd52b..068d617a 100644 --- a/lib/corpus.py +++ b/lib/corpus.py @@ -407,14 +407,21 @@ class Corpus: best_i = -1 best_score = -1e9 picked_mat = self.clip_embeddings[np.array(picked)] - for i in remaining: + # Normalize the position term to [0, 1] so it lives on the + # same scale as cosine similarity. An absolute index grows + # with the pool, drowning the similarity term: at the 0.5 + # default a candidate one slot later needed a similarity gap + # > 1.0 to be preferred — impossible for non-negative + # cosines — so diversify() degenerated to input order. + denom = max(1, len(remaining) - 1) + for pos, i in enumerate(remaining): sim_picked = float(np.max(self.clip_embeddings[i] @ picked_mat.T)) # We want LOW similarity, so we negate. score = -sim_picked # Diversity weights how hard we penalize similarity. At # diversity=1 we always pick the most different; at # diversity=0 we just take them in input order. - score = diversity * score + (1.0 - diversity) * (-remaining.index(i)) + score = diversity * score + (1.0 - diversity) * (-pos / denom) if score > best_score: best_score = score best_i = i diff --git a/tests/lib/test_corpus_diversify_scale.py b/tests/lib/test_corpus_diversify_scale.py new file mode 100644 index 00000000..169b3fe3 --- /dev/null +++ b/tests/lib/test_corpus_diversify_scale.py @@ -0,0 +1,126 @@ +"""Regression tests: Corpus.diversify() must actually diversify at its default. + +The greedy score mixed two incommensurate scales: + + score = diversity * (-sim_picked) + (1 - diversity) * (-remaining.index(i)) + +`sim_picked` is a cosine in [-1, 1]; `remaining.index(i)` is an absolute +list position, unbounded. At the default diversity=0.5 a candidate one +slot later needed a similarity gap > 1.0 to be preferred — impossible for +non-negative cosines — so diversify() returned the input order verbatim, +even when the list held exact duplicates. Its one documented job ("make +sure no two adjacent slots are visually redundant") never happened, and +the effective meaning of `diversity` changed with pool size. + +The fix normalizes the position term to [0, 1] so both terms share a +scale. diversity=0 still returns input order; diversity=1 still picks the +most mutually dissimilar. +""" + +import sys +from pathlib import Path + +import numpy as np +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(PROJECT_ROOT)) + +from lib.corpus import EMBED_DIM, ClipRecord, Corpus + + +def _corpus(vectors: list[list[float]]) -> Corpus: + """Build an in-memory corpus from small L2-normalised vectors.""" + corp = Corpus(Path("/nonexistent")) + emb = np.zeros((len(vectors), EMBED_DIM), dtype=np.float32) + for row, vec in enumerate(vectors): + emb[row, : len(vec)] = vec + emb[row] /= np.linalg.norm(emb[row]) + corp.clip_embeddings = emb + corp.records = [ + ClipRecord( + clip_id=f"c{row}", + source="test", + source_id=str(row), + source_url="", + local_path="", + ) + for row in range(len(vectors)) + ] + corp._id_to_row = {r.clip_id: i for i, r in enumerate(corp.records)} + return corp + + +def test_default_diversity_separates_exact_duplicates(): + # c1 is an exact duplicate of c0; c2/c3 are orthogonal to both. + corp = _corpus([[1, 0, 0], [1, 0, 0], [0, 1, 0], [0, 0, 1]]) + + kept = corp.diversify(["c0", "c1", "c2", "c3"], n=3) + + # The duplicate must not sit adjacent to its twin at the default. + assert kept[0] == "c0" + assert kept[1] != "c1" + + +def test_default_diversity_is_not_a_noop(): + # Three exact duplicates (c0, c2, c5) interleaved with distinct clips. + # The old scoring returned the input order verbatim here. + corp = _corpus( + [ + [1, 0, 0], + [0.8, 0.6, 0], + [1, 0, 0], + [0.6, 0.8, 0], + [0, 1, 0], + [1, 0, 0], + ] + ) + ids = [f"c{i}" for i in range(6)] + + kept = corp.diversify(ids, n=6) + + assert kept != ids # reorders when the input holds duplicates + for a, b in zip(kept, kept[1:]): # and no two adjacent slots are twins + va = corp.clip_embeddings[corp._id_to_row[a]] + vb = corp.clip_embeddings[corp._id_to_row[b]] + assert float(va @ vb) < 0.999 + + +def test_diversity_semantics_do_not_collapse_with_pool_size(): + # Duplicate of c0 right at position 1, moderately-related fillers, + # one orthogonal clip at the end. With the unnormalised position + # term the duplicate always won slot 2 at diversity=0.5 because the + # positional penalty scaled with the pool. + half_sqrt3 = np.sqrt(3) / 2 + vectors = [[1, 0, 0], [1, 0, 0]] + [[0.5, half_sqrt3, 0]] * 8 + [[0, 0, 1]] + corp = _corpus(vectors) + ids = [f"c{i}" for i in range(len(vectors))] + + kept = corp.diversify(ids, n=3, diversity=0.5) + + assert kept[0] == "c0" + assert "c1" not in kept[:2] # the exact duplicate no longer wins slot 2 + + +def test_diversity_zero_keeps_input_order(): + corp = _corpus([[1, 0, 0], [1, 0, 0], [0, 1, 0], [0, 0, 1]]) + ids = ["c0", "c1", "c2", "c3"] + + assert corp.diversify(ids, n=4, diversity=0.0) == ids + + +def test_diversity_one_picks_most_dissimilar(): + # c1 duplicates c0; c2 and c3 are mutually orthogonal. + corp = _corpus([[1, 0, 0], [1, 0, 0], [0, 1, 0], [0, 0, 1]]) + + kept = corp.diversify(["c0", "c1", "c2", "c3"], n=3, diversity=1.0) + + assert kept == ["c0", "c2", "c3"] + + +def test_empty_and_unknown_ids(): + corp = _corpus([[1, 0, 0]]) + + assert corp.diversify([], n=3) == [] + assert corp.diversify(["nope"], n=3) == [] + assert corp.diversify(["c0", "nope"], n=3) == ["c0"]