fix(engine): normalize whitespace in candidate entity names at resolution intake - #3338
Merged
Merged
Conversation
…#3275) Extraction can hand back entity names carrying embedded newlines/tabs, which are then stored verbatim as entities.canonical_name and shear every line-oriented consumer (psql -A output, log lines, exports). Collapse whitespace runs to a single space and strip the ends at _prepare_entities_for_resolution -- the single choke point both entity resolution entry paths funnel through, and before the flat list / entity_to_unit mapping is derived, so the resolver's positional invariant is untouched. Case is left alone: the registry already matches on LOWER(canonical_name). Two consequences handled at the same spot: - a candidate that is empty after normalization is dropped instead of being created as an entity with a blank canonical_name (the resolver has no guard of its own); - candidates that normalization makes identical are deduplicated per fact, so the same entity is not resolved twice and its mention_count bumped twice (the upstream dedup in entity_processing runs on the raw text). Existing rows are not migrated: renormalizing a stored name can collide with the (bank_id, LOWER(canonical_name)) uniqueness, so cleaning them up is a merge, not an UPDATE.
JoshFunnell
added a commit
to JoshFunnell/hindsight
that referenced
this pull request
Aug 11, 2026
Upstream merged the whitespace-normalization half of this PR as vectorize-io#3338 (44b597c) and went further, adding per-fact case-insensitive dedupe and an empty-candidate drop. Upstream's implementation is taken wholesale; only this branch's unique contribution is re-applied on top: skipping tag-shaped category-label candidates (domain:lens) that extraction was minting as entities. Check order is load-bearing: normalize -> empty-drop -> tag-shape -> dedupe. The tag-shape skip must precede dedupe so a skipped tag never enters seen_in_fact and every occurrence increments the counter; after dedupe a repeat tag would be silently deduped instead of counted. Configured entity labels (use:use-001) stay exempt via is_label_entity and still reach resolution. Our normalization tests are dropped as duplicates of upstream's; the tag-shape tests are kept.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3275. Supersedes the first half of #3277.
The defect
Extraction sometimes hands back candidate entity names carrying embedded newlines/tabs. They are stored verbatim as
entities.canonical_name, and every line-oriented consumer then shears:psql -Asplits one row across lines, log lines break, exports corrupt.The fix
Whitespace runs (including
\n,\r,\t) collapse to a single space and the ends are stripped, in_prepare_entities_for_resolution(hindsight-api-slim/hindsight_api/engine/retain/link_utils.py) — the single choke point both entity-resolution entry paths funnel through (retain viaentity_processing.resolve_entities, and the memory-edit path inMemoryEngine).It runs before the flat-list /
entity_to_unitmapping is derived, so the resolver's positional invariant (output index-aligned with input) is untouched. Case is deliberately left alone — the registry already matches onLOWER(canonical_name), so lowercasing here would only destroy the display form.Two consequences of normalizing are handled at the same spot:
" \n "normalizes to"", and the resolver has no empty-name guard of its own (entity_resolver.pytakesentity_data["text"]straight through to create), so without this an entity with a blankcanonical_namegets created.entity_processing.pyruns on the raw text, so"Acme\nCorp"(from extraction) and"Acme Corp"(from the caller's owncontent.entities) both survive it and collide only after normalization — resolving the same entity twice for one fact and bumping itsmention_counttwice. The dedup is scoped to a single fact, so the same entity mentioned by two facts is still resolved for each.Not in scope
Existing rows are not migrated. Renormalizing a stored name can collide with the
(bank_id, LOWER(canonical_name))uniqueness, so cleaning up already-stored names is a merge operation, not anUPDATE— left to operators / a follow-up.Tests
hindsight-api-slim/tests/test_entity_name_hygiene.py— 22 tests over the pure normalizer and the intake function: normalization of both dict- and object-style candidates,nearby_entitiescarrying the normalized names, empty/whitespace-only drops, per-fact dedup (case-insensitive, first spelling wins, not applied across facts), andentity_to_unit/event_datestaying index-aligned after a drop.Local regression on the entity suites (
test_link_utils,test_entity_intrabatch_dedup,test_entity_resolver,test_entity_labels,test_retain_orchestrator_mapping,test_entity_name_hygiene): 170 passed.