From 3a5dee3bd00ac168436b47545505a77684367eb8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 24 Aug 2026 08:42:28 +0900 Subject: [PATCH] fix(lineage): clamp embedding cosine instead of remapping it Devin Review on this PR flagged that cosine_similarity's (cosine+1)/2 remap silently defeats the fix: real sentence embeddings are anisotropic, so an unrelated pair's near-zero raw cosine gets inflated to ~0.5-0.65, which combined with any temporal proximity can still clear DEFAULT_MIN_FUSED_SCORE and reproduce the exact false-positive edge this PR set out to close. Clamp the raw cosine into [0, 1] instead, matching the unremapped STS evaluation convention (Reimers & Gurevych, 2019) this channel already cites. Replaced the regression test's unrealistic exactly-opposite-vector fixture with a low but positive cosine (0.05), grounded in embedding anisotropy (Ethayarajh, 2019), and added direct cosine_similarity clamping unit tests. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01491gBgBW98rZ7dm2fiTwF3 --- .../2.24.0-lineage-text-channel-embeddings.md | 5 ++- CHANGELOG.md | 4 ++- ...190-lineage-text-channel-embedding-swap.md | 36 +++++++++++++++++++ lineageweave/embedding_client.py | 19 ++++++++-- tests/test_embedding_client_edges.py | 17 +++++++++ tests/test_reconstruct.py | 19 +++++++--- 6 files changed, 92 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.d/2.24.0-lineage-text-channel-embeddings.md b/CHANGELOG.d/2.24.0-lineage-text-channel-embeddings.md index f010672dc..ea09e82e4 100644 --- a/CHANGELOG.d/2.24.0-lineage-text-channel-embeddings.md +++ b/CHANGELOG.d/2.24.0-lineage-text-channel-embeddings.md @@ -8,4 +8,7 @@ embedding provider is configured, falling back to the prior `difflib.SequenceMatcher` behavior otherwise (ADR 0190). Threaded through `rebuild_lineage`, the analysis-run worker, and - `scripts/import_postgresql_posts.py`. + `scripts/import_postgresql_posts.py`. Cosine similarity is clamped into + `[0, 1]`, not remapped from `[-1, 1]`: the remap would have inflated a + genuinely unrelated pair's near-zero real-world cosine into a false + "weak positive" that still clears the fusion floor. diff --git a/CHANGELOG.md b/CHANGELOG.md index efedae955..a399ef053 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,9 @@ All notable changes to this project are documented here. Format follows falling back to character-overlap matching -- reduces the case where two posts about unrelated topics that happen to share common words or sentence structure get linked as parent/child within a coarsely-grouped - bucket (ADR 0190). + bucket (ADR 0190). Cosine similarity is clamped into `[0, 1]`, not + remapped from `[-1, 1]`, since real embeddings never place a genuinely + unrelated pair near the fully-opposite end of that range. - Corpus-wide lineage rebuilds now run synchronous orchestrator adjudication off the API event-loop thread and before the short atomic projection-write transaction. A temporary adjudication failure leaves the existing graph diff --git a/docs/adr/0190-lineage-text-channel-embedding-swap.md b/docs/adr/0190-lineage-text-channel-embedding-swap.md index d5d62e3fc..4cf5425b2 100644 --- a/docs/adr/0190-lineage-text-channel-embedding-swap.md +++ b/docs/adr/0190-lineage-text-channel-embedding-swap.md @@ -73,6 +73,35 @@ ADR wires was staged capability, not new infrastructure. `scripts/import_postgresql_posts.py` passes the `embedding_client` it already builds for post-content embedding into `rebuild_lineage` too. +## Amendment (2026-08-24): clamp, don't remap, the raw cosine + +Post-review finding (Devin Review on this PR): the initial implementation +mapped `cosine_similarity`'s output via `(cosine + 1) / 2`, the textbook +transform for a similarity measure that spans the full `[-1, 1]` range. Real +sentence embeddings do not: they occupy an anisotropic cone (Ethayarajh, +2019), so two genuinely unrelated short texts from an actual provider score +a modestly *positive* raw cosine in practice, essentially never near -1. The +remap inflated that unrelated baseline to roughly 0.5-0.65 -- a "weak +positive" channel score that, combined with any temporal proximity, could +still clear `DEFAULT_MIN_FUSED_SCORE` and reproduce the exact false-positive +edge this ADR set out to close, just through the embedding channel instead +of difflib. Fixed by clamping the raw cosine into `[0, 1]` +(`max(0.0, min(1.0, cosine))`) instead of remapping it, matching how cosine +similarity is used unremapped in the STS evaluation convention (Reimers & +Gurevych, 2019) already cited above. + +This also surfaced a distinct, still-open observation worth flagging rather +than silently absorbing into this fix: for two records roughly an hour +apart, `temporal_score` alone (`1 / (1 + gap_days)`) already contributes +close to `DEFAULT_MIN_FUSED_SCORE` on its own once weights renormalize +without an `llm` channel, so *any* weakly-positive text score -- clamped +cosine included -- can still tip a temporally-close, topically-unrelated +pair over the floor. Flagged here rather than in +`docs/product-technical-gap-baseline.md` -- this branch predates that +document's current structure; carry this observation forward on the next +rebase against `main`. Changing `temporal_score`'s steepness or +`DEFAULT_MIN_FUSED_SCORE` is a calibration decision outside this PR's scope. + ## Consequences - When an embedding provider is configured (already true in this @@ -102,6 +131,13 @@ ADR wires was staged capability, not new infrastructure. ## References — APA 7th +Ethayarajh, K. (2019). How contextual are contextualized word +representations? Comparing the geometry of BERT, ELMo, and GPT-2 embeddings. +In *Proceedings of the 2019 Conference on Empirical Methods in Natural +Language Processing and the 9th International Joint Conference on Natural +Language Processing (EMNLP-IJCNLP)* (pp. 55-65). Association for +Computational Linguistics. https://doi.org/10.18653/v1/D19-1006 + Reimers, N., & Gurevych, I. (2019). Sentence-BERT: Sentence embeddings using Siamese BERT-networks. In *Proceedings of the 2019 Conference on Empirical Methods in Natural Language Processing and the 9th International Joint diff --git a/lineageweave/embedding_client.py b/lineageweave/embedding_client.py index d76b12944..785b1b4d2 100644 --- a/lineageweave/embedding_client.py +++ b/lineageweave/embedding_client.py @@ -148,14 +148,29 @@ def orchestrator_embedding_client(base_url: str, api_key: str, model: str): def cosine_similarity(a: list[float], b: list[float]) -> float: - """Cosine similarity mapped from ``[-1, 1]`` into the ``[0, 1]`` channel range.""" + """Cosine similarity clamped into the ``[0, 1]`` channel range. + + Sentence embeddings occupy an anisotropic cone rather than spreading + across the full unit sphere (Ethayarajh, 2019): two genuinely unrelated + short texts from a real provider routinely score a modestly *positive* + raw cosine (roughly 0.0-0.3 in practice), never near -1. Remapping via + ``(cosine + 1) / 2`` -- the textbook transform for a similarity measure + that actually spans the full range -- inflates that unrelated baseline + to roughly 0.5-0.65, which silently defeats + ``reconstruct.DEFAULT_MIN_FUSED_SCORE``: an unrelated pair's "weak + positive" channel score, combined with any temporal proximity, can + still clear the floor. Clamping the raw cosine instead (never remapping + it) keeps unrelated pairs near their true low score and matches how + cosine similarity is used, unremapped, in the STS evaluation convention + (Reimers & Gurevych, 2019) this channel is otherwise built to. + """ dot = sum(x * y for x, y in zip(a, b)) norm_a = math.sqrt(sum(x * x for x in a)) norm_b = math.sqrt(sum(y * y for y in b)) if norm_a == 0.0 or norm_b == 0.0: return 0.0 cosine = dot / (norm_a * norm_b) - return (cosine + 1.0) / 2.0 + return max(0.0, min(1.0, cosine)) def chunked_max_similarity( diff --git a/tests/test_embedding_client_edges.py b/tests/test_embedding_client_edges.py index a7d8a9fcb..2971d5481 100644 --- a/tests/test_embedding_client_edges.py +++ b/tests/test_embedding_client_edges.py @@ -100,3 +100,20 @@ def embed(self, text: str) -> list[float]: def test_cosine_similarity_returns_zero_for_zero_vector() -> None: assert embedding_client.cosine_similarity([0.0], [1.0]) == 0.0 + + +def test_cosine_similarity_returns_raw_value_not_a_remap() -> None: + """Regression: cosine_similarity must clamp, not remap (cosine+1)/2. + + A remap turns a genuinely unrelated pair's near-zero raw cosine into a + false "weak positive" ~0.5 that clears reconstruct's fusion floor (see + lineageweave/embedding_client.py::cosine_similarity docstring). Orthogonal + vectors (cosine 0.0) must stay at 0.0, not become 0.5. + """ + assert embedding_client.cosine_similarity([1.0, 0.0], [0.0, 1.0]) == 0.0 + assert embedding_client.cosine_similarity([1.0, 0.0], [1.0, 0.0]) == 1.0 + + +def test_cosine_similarity_clamps_a_negative_cosine_to_zero() -> None: + """Opposite vectors (cosine -1.0) must clamp to 0.0, not go negative.""" + assert embedding_client.cosine_similarity([1.0, 0.0], [-1.0, 0.0]) == 0.0 diff --git a/tests/test_reconstruct.py b/tests/test_reconstruct.py index 03ec36c27..56a0e5433 100644 --- a/tests/test_reconstruct.py +++ b/tests/test_reconstruct.py @@ -114,9 +114,19 @@ def test_embedding_channel_overrides_a_difflib_false_positive() -> None: ratio -- high enough, combined with temporal closeness, to clear DEFAULT_MIN_FUSED_SCORE and force a spurious parent-child Event Lineage edge between unrelated posts (see docs/product-technical-gap-baseline.md). - A real embedding channel judging the two topics as dissimilar (mapped - here to opposite unit vectors, cosine similarity 0.0) must be able to - keep them apart instead. + + The stub vectors below give a raw cosine of 0.05 -- a low, but not + artificially perfect, positive similarity -- rather than exactly + opposite (-1.0) unit vectors. Real sentence embeddings occupy an + anisotropic cone (Ethayarajh, 2019): two genuinely unrelated texts from + an actual provider essentially never score near -1, so a fixture that + only proves the fix works at that unrealistic extreme would not have + caught the (cosine+1)/2 remap bug this test also guards against (see + lineageweave/embedding_client.py::cosine_similarity) -- that remap would + have turned even a true -1.0 into 0.0, same as this clamp does, but it + would *also* have turned this test's realistic 0.05 into a + floor-clearing ~0.525. Clamping instead of remapping is what keeps this + pair apart. """ budget_label = "Quarterly budget review for the northern region team" safety_label = "Quarterly safety review for the northern region plant" @@ -133,7 +143,8 @@ def test_embedding_channel_overrides_a_difflib_false_positive() -> None: "the false-positive link this test's embedding channel must prevent" ) - stub = _StubEmbeddingClient({budget_label: [1.0, 0.0], safety_label: [-1.0, 0.0]}) + low_similarity_vector = [0.05, 0.9987492177719986] # cosine 0.05 against [1.0, 0.0] + stub = _StubEmbeddingClient({budget_label: [1.0, 0.0], safety_label: low_similarity_vector}) with_embedding = reconstruct(records, embedding=stub) assert with_embedding[0].edges == [] assert "r2" in with_embedding[0].roots