Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 57 additions & 2 deletions hindsight-api-slim/hindsight_api/engine/retain/link_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""

import logging
import re
import time
from datetime import UTC

Expand All @@ -24,6 +25,24 @@
# Sentinel UUID used in the unique index to represent NULL entity_id
_NIL_ENTITY_UUID = "00000000-0000-0000-0000-000000000000"

# Any run of whitespace, including the \n / \r / \t that extraction sometimes
# leaves inside a candidate entity name.
_WHITESPACE_RUN_RE = re.compile(r"\s+")


def _normalize_entity_name(name: str) -> str:
"""Collapse internal whitespace runs to a single space and strip the ends.

Extraction can hand back names carrying embedded newlines/tabs, which then
become ``entities.canonical_name`` values that shear every line-oriented
consumer (``psql -A`` output, log lines, exports) — issue #3275. Case is
deliberately untouched: the entity registry already matches on
``LOWER(canonical_name)``, so lowercasing here would only lose the display
form.
"""
return _WHITESPACE_RUN_RE.sub(" ", name).strip()


# Maximum number of temporal links to keep per unit (from_unit_id).
# Retrieval only reads top 10-20 per unit via LATERAL join, so keeping
# more is wasted storage and write amplification.
Expand Down Expand Up @@ -151,6 +170,12 @@ def _prepare_entities_for_resolution(
"""
Convert LLM entities into the flat format expected by entity resolver.

Candidate names are whitespace-normalized here (see ``_normalize_entity_name``)
and names that are empty afterwards are dropped, so no downstream stage has to
cope with an entity whose canonical name is blank or spans several lines.
Both happen before the flat list and ``entity_to_unit`` are derived, keeping
the resolver's positional invariant (output index-aligned with input) intact.

Returns:
Tuple of (all_entities_flat, all_entities, entity_to_unit) where:
- all_entities_flat: flat list of entity dicts ready for resolve_entities_batch
Expand All @@ -159,15 +184,45 @@ def _prepare_entities_for_resolution(
"""
substep_start = time.time()
all_entities = []
dropped_empty = 0
for entity_list in llm_entities:
formatted_entities = []
# Normalization can make two candidates that reached here as distinct
# strings ("Acme\nCorp" from extraction, "Acme Corp" from the caller's
# own entity list) identical, and the upstream dedup in
# entity_processing runs on the raw text. Without this, the same entity
# would be resolved twice for one fact and its mention_count bumped twice.
seen_in_fact: set[str] = set()
for ent in entity_list:
if hasattr(ent, "text"):
formatted_entities.append({"text": ent.text, "type": "CONCEPT"})
raw_text, entity_type = ent.text, "CONCEPT"
elif isinstance(ent, dict):
formatted_entities.append({"text": ent.get("text", ""), "type": ent.get("type", "CONCEPT")})
raw_text, entity_type = ent.get("text", ""), ent.get("type", "CONCEPT")
else:
continue

normalized_text = _normalize_entity_name(raw_text)
if not normalized_text:
# A blank or whitespace-only candidate would otherwise be created
# as an entity with an empty canonical_name — the resolver has no
# guard of its own.
dropped_empty += 1
continue

if normalized_text.lower() in seen_in_fact:
continue
seen_in_fact.add(normalized_text.lower())

formatted_entities.append({"text": normalized_text, "type": entity_type})
all_entities.append(formatted_entities)

if dropped_empty:
_log(
log_buffer,
f" [6.1] Dropped {dropped_empty} empty candidate entity name(s)",
level="debug",
)

total_entities = sum(len(ents) for ents in all_entities)
_log(
log_buffer,
Expand Down
175 changes: 175 additions & 0 deletions hindsight-api-slim/tests/test_entity_name_hygiene.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
"""Candidate entity-name hygiene at resolution intake (issue #3275).

``_prepare_entities_for_resolution`` is the single choke point both entity
resolution entry paths funnel through (retain via
``entity_processing.resolve_entities``, and the memory-edit path in
``MemoryEngine``), so it is where a name is made safe to store:

1. whitespace runs — including the ``\\n`` extraction sometimes leaves behind —
collapse to a single space, and the ends are stripped;
2. a name that is empty afterwards is dropped rather than stored as an entity
with a blank ``canonical_name``;
3. candidates that normalization made identical are deduplicated per fact.

All of it runs before the flat list / ``entity_to_unit`` mapping is derived, so
the resolver's positional invariant is untouched.
"""

import pytest

from hindsight_api.engine.retain.link_utils import (
_normalize_entity_name,
_prepare_entities_for_resolution,
)


class _FakeEntity:
"""Object-style candidate: exposes ``.text``, like the extraction models."""

def __init__(self, text: str):
self.text = text


def _texts(all_entities_flat: list[dict]) -> list[str]:
return [e["text"] for e in all_entities_flat]


def _prepare(entities: list, unit_ids: list[str] | None = None):
"""Run one fact's candidate list through intake."""
return _prepare_entities_for_resolution(
unit_ids=unit_ids or ["u1"],
sentences=["fact text"],
fact_dates=[None],
llm_entities=[entities],
)


# --- _normalize_entity_name ---


@pytest.mark.parametrize(
"raw,expected",
[
("Acme\nCorp", "Acme Corp"),
("a\r\n b\tc", "a b c"),
(" leading and trailing ", "leading and trailing"),
("multiple spaces inside", "multiple spaces inside"),
("Normal Name", "Normal Name"),
("", ""),
(" \n\t ", ""),
],
)
def test_normalize_entity_name(raw, expected):
assert _normalize_entity_name(raw) == expected


def test_normalize_entity_name_preserves_case():
# The registry matches on LOWER(canonical_name); lowercasing here would only
# destroy the display form.
assert _normalize_entity_name("MiXeD\nCaSe") == "MiXeD CaSe"


def test_normalize_entity_name_leaves_ordinary_punctuation_alone():
assert _normalize_entity_name("Dr. Foo-Bar (ACME), Inc.") == "Dr. Foo-Bar (ACME), Inc."


# --- intake: normalization reaches the text handed to the resolver ---


def test_intake_normalizes_dict_style_candidates():
all_entities_flat, _all, _map = _prepare([{"text": "Acme\nCorp", "type": "ORG"}])
assert _texts(all_entities_flat) == ["Acme Corp"]
assert all_entities_flat[0]["type"] == "ORG"


def test_intake_normalizes_object_style_candidates():
all_entities_flat, _all, _map = _prepare([_FakeEntity("a\r\n b\tc")])
assert _texts(all_entities_flat) == ["a b c"]


def test_intake_normalizes_nearby_entities_too():
# nearby_entities is the co-occurrence signal the resolver scores against;
# it must carry the normalized names, not the raw ones.
all_entities_flat, all_entities, _map = _prepare(
[{"text": "Acme\nCorp", "type": "CONCEPT"}, {"text": "Alice", "type": "CONCEPT"}]
)
assert [e["text"] for e in all_entities[0]] == ["Acme Corp", "Alice"]
assert [e["text"] for e in all_entities_flat[1]["nearby_entities"]] == ["Acme Corp", "Alice"]


# --- intake: empty names are dropped, not stored blank ---


@pytest.mark.parametrize("raw", ["", " ", "\n", " \t\r\n "])
def test_intake_drops_empty_and_whitespace_only_candidates(raw):
all_entities_flat, all_entities, entity_to_unit = _prepare([{"text": raw, "type": "CONCEPT"}])
assert all_entities_flat == []
assert all_entities == [[]]
assert entity_to_unit == []


def test_intake_drops_candidate_dict_without_text_key():
all_entities_flat, _all, _map = _prepare([{"type": "CONCEPT"}, {"text": "Alice", "type": "CONCEPT"}])
assert _texts(all_entities_flat) == ["Alice"]


def test_intake_keeps_real_entities_alongside_dropped_empties():
all_entities_flat, _all, entity_to_unit = _prepare(
[{"text": " ", "type": "CONCEPT"}, {"text": " Alice ", "type": "CONCEPT"}]
)
assert _texts(all_entities_flat) == ["Alice"]
# entity_to_unit stays index-aligned with the flat list the resolver receives.
assert entity_to_unit == [("u1", 0, None)]


# --- intake: dedup of candidates that normalization made identical ---


def test_intake_dedupes_candidates_normalization_made_identical():
# The upstream dedup in entity_processing runs on raw text, so these two
# arrive here distinct and collide only after normalization.
all_entities_flat, all_entities, entity_to_unit = _prepare(
[{"text": "Acme\nCorp", "type": "CONCEPT"}, {"text": "Acme Corp", "type": "CONCEPT"}]
)
assert _texts(all_entities_flat) == ["Acme Corp"]
assert [e["text"] for e in all_entities[0]] == ["Acme Corp"]
assert len(entity_to_unit) == 1


def test_intake_dedupe_is_case_insensitive_and_keeps_first_spelling():
all_entities_flat, _all, _map = _prepare(
[{"text": "Acme Corp", "type": "CONCEPT"}, {"text": "acme\ncorp", "type": "CONCEPT"}]
)
assert _texts(all_entities_flat) == ["Acme Corp"]


def test_intake_dedupe_is_scoped_per_fact():
# The same entity mentioned by two different facts must still be resolved
# for each of them — the dedup is within a fact, not across the batch.
all_entities_flat, _all, entity_to_unit = _prepare_entities_for_resolution(
unit_ids=["u1", "u2"],
sentences=["first", "second"],
fact_dates=[None, None],
llm_entities=[
[{"text": "Acme\nCorp", "type": "CONCEPT"}],
[{"text": "Acme Corp", "type": "CONCEPT"}],
],
)
assert _texts(all_entities_flat) == ["Acme Corp", "Acme Corp"]
assert [unit_id for unit_id, _idx, _date in entity_to_unit] == ["u1", "u2"]


def test_intake_attaches_fact_dates_after_dropping():
from datetime import UTC, datetime

when = datetime(2026, 8, 8, tzinfo=UTC)
all_entities_flat, _all, _map = _prepare_entities_for_resolution(
unit_ids=["u1"],
sentences=["fact text"],
fact_dates=[when],
llm_entities=[[{"text": " ", "type": "CONCEPT"}, {"text": "Alice", "type": "CONCEPT"}]],
)
# The event_date attach loop walks entity_to_unit positionally; a dropped
# candidate must not shift it.
assert _texts(all_entities_flat) == ["Alice"]
assert all_entities_flat[0]["event_date"] == when