diff --git a/hindsight-api-slim/hindsight_api/engine/retain/link_utils.py b/hindsight-api-slim/hindsight_api/engine/retain/link_utils.py index 690e46872..d4315e519 100644 --- a/hindsight-api-slim/hindsight_api/engine/retain/link_utils.py +++ b/hindsight-api-slim/hindsight_api/engine/retain/link_utils.py @@ -3,6 +3,7 @@ """ import logging +import re import time from datetime import UTC @@ -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. @@ -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 @@ -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, diff --git a/hindsight-api-slim/tests/test_entity_name_hygiene.py b/hindsight-api-slim/tests/test_entity_name_hygiene.py new file mode 100644 index 000000000..798e6dbd4 --- /dev/null +++ b/hindsight-api-slim/tests/test_entity_name_hygiene.py @@ -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