SIGN IN SIGN UP

fix(entities): don't merge a name onto an existing entity on history alone, + a resolution eval (#3752)

* docs: explain how entity resolution decides to merge a name

Entity resolution's thresholds were documented one env var at a time, with
nothing describing how the pieces fit together — so the obvious reading is
that pg_trgm similarity is what decides a match. It isn't: the trigram
threshold only admits candidates, and the merge decision is a separate
weighted score (sequence-ratio name similarity 0.5 + co-occurrence 0.3 +
recency 0.2, merged above 0.6) with no minimum name similarity of its own.
The two measures disagree most on short names, which is how a new person's
facts can land on an unrelated existing entity.

- configuration.md: new "How entity resolution decides" section covering both
  stages, the weights, and what to change in either direction.
- configuration.md: RETAIN_ENTITY_LOOKUP said `full` was "exact match". It is
  exact-or-substring, and both strategies feed the same scoring pass, so the
  choice changes which names can merge — not just how fast the lookup is.
- retain.md: short note that resolution can absorb a new name into an existing
  entity, pointing at the reference section.

* fix(entities): don't merge a name onto an existing entity on history alone (#3751)

A retain that correctly extracted a new person, Tigran, stored the fact on a
pre-existing country entity, Iran. Three defects combined:

1. The metric that admits candidates and the metric that decides the merge were
   different, and they rank differently. pg_trgm scores tigran/iran at 0.20 —
   above the 0.15 that admits it, below anything that should merge. difflib's
   SequenceMatcher, which actually decides, scores it 0.80: higher than
   alice/alice chen (0.67), a merge the resolver exists to make. With the
   non-name signals worth 0.5 of the 0.6 threshold and no floor of their own,
   history alone could carry it.

   Gate candidates on trigram similarity before scoring
   (HINDSIGHT_API_ENTITY_MERGE_MIN_SIMILARITY, 0.3). A gate, not a replacement:
   SequenceMatcher stays load-bearing for typo variants arriving with no
   co-occurrence context ("Dr Waler" -> "Dr Wall"), which is why the metric is
   not simply swapped. This gives the existing-entity side the same three-tier
   story the in-batch side already had — 0.15 to be considered, 0.3 to be merged
   onto, 0.5 to be folded in-batch.

2. The co-occurrence term counted every shared entity equally, so `user` — which
   co-occurs with nearly everything in a mature bank — was worth as much as a
   selective partner, and was what pushed Iran over. Weight each shared partner
   by 1/sqrt(degree). Degrees come from the co-occurrence rows already fetched,
   so no extra query; with no degrees known the formula is arithmetically the old
   overlap fraction, so nothing changes for callers that pass none.

3. A score *of* 0.6 landed on either side of the strict `>` depending only on
   which signals produced it: 0.4 + 0.2 is 0.6000000000000001 and merged, 0.3 +
   0.3 is 0.6 and did not. Round, and compare inclusively — which is the verdict
   the common composition already got, so no merge that happens today stops.

Also folds the three byte-identical co-occurrence-map builders into one indexer
(it now computes degrees too), and shares the trigram Jaccard so the mention's
trigram set is built once per mention rather than once per candidate — the
scoring loop runs up to max_candidates times per mention (GH-3211).

* test(entities): dataset-driven eval of the whole resolution pipeline

The unit tests around entity resolution pin individual pieces — a similarity
number, one scoring branch, one strategy's SQL. #3751 was not any of those: each
piece behaved as designed and the *combination* attributed a new person's facts
to an unrelated country. Nothing was positioned to catch that.

25 cases, each seeding real entities and entity_cooccurrences into a bank and
running the real resolve_entities_batch over them — pg_trgm probe, the partial
index that excludes labels, the candidate cap, scoring, insert. Covers identity
(case, Turkish İ, emoji), variants that must merge, coincidental names that must
not, co-occurrence quality, the recency window, labels, caller-authored names,
in-batch clustering and containment.

Every case runs under both lookup strategies and must agree, unless it declares
otherwise — which turns "full vs trigram is not just a performance choice" from
a claim in the docs into six cases that say exactly where they diverge.

Time is fixed (last_seen is seeded relative to a constant EVENT_DATE), and the
resolver is built from the shipped config rather than constructor defaults, so
the dataset measures a real deployment and moving a threshold shows up here.
Verified it bites: with HINDSIGHT_API_ENTITY_MERGE_MIN_SIMILARITY=0.01 the three
collision cases fail exactly as reported, and only under trigram.

Two cases are xfailed as known limitations rather than quietly given the answer
the code produces:

- Iran/Iraq clears the floor at 0.43, so a genuinely selective shared partner
  still merges two distinct entities. Telling "same entity, different spelling"
  from "different entity, similar spelling" needs the extractor's type or an
  adjudication step, not another threshold.
- A decorated form of a stored name ("Wren 🎵" vs "Wren") has *identical*
  trigram sets, but the score reads the sequence ratio (0.80), so it forks into
  a second entity once the stored one is a day stale — while the same two forms
  DO unify when both are new, since the in-batch pass merges at 0.5 trigram.
  #3107 fixed that half only.

* test(entities): move the resolution eval to JSONL and grow it to 61 cases

The dataset was Python literals inside the test module, which put the cases and
the harness in one file and made adding a case a code change. It now lives in
tests/entity_resolution_cases.jsonl, one object per line, and the module is only
a loader plus the runner.

61 passing cases (up from 23), 3 recorded as known limitations. New coverage:
diacritics across four scripts, punctuation as a pg_trgm separator (hyphen,
apostrophe, trailing period), spelling variants, typos, abbreviations, middle
initials, corporate suffixes, near-miss place and person names, the recency
window from both sides, the floor's inclusive boundary (Nick/Nicolas is exactly
0.30), label keys and unseen values, three-way in-batch clustering, containment
in both directions, a crowded candidate field, and bank isolation.

Two things the dataset now guards that it did not before:

- A well-formedness test. A case that co-occurs with a name it never seeded, or
  expects the wrong number of answers, would pass while measuring nothing — how
  the first draft of this eval nearly shipped blind. It also fails if the file
  drops below 50 passing cases.
- The bank-isolation case seeds the other bank with exactly the entity the
  mention would merge onto, so a leak changes the answer instead of being masked
  by an identical name.

Verified the dataset discriminates in both directions: with
ENTITY_MERGE_MIN_SIMILARITY=0.01 five cases fail (the collisions, one of them
under `full` too, since "Hannah" contains "anna"), and at 0.6 twenty fail as
legitimate variants stop merging. So it brackets the setting rather than only
pinning today's value.

The third known limitation is unchanged in kind but worse than recorded: with
the stored entity seen the same day, Iran/Iraq merges on an *indiscriminate*
partner too, not only a selective one — 0.75 by sequence ratio is 0.375 and
recency alone adds 0.2. The damping case now holds the stored entity outside the
recency window so it pins the damping rather than the clock.

* fix(entities): close the two defects the resolution eval surfaced

Both came out of the dataset in the previous commit, where they were recorded as
known limitations rather than given whatever answer the code produced.

**Different given names sharing a surname merged.** "John Smith" and "Jane
Smith" are 0.47 by trigram and 0.80 by sequence ratio, so two people who share a
surname and a workplace scored as one entity — the shared long word drowns out a
completely different short one. A candidate must now agree word by word: every
word of the shorter name has to find a counterpart in the longer one, by
equality, by abbreviation (corp/corporation, via prefix) or by spelling
(são/sao 0.67, waler/wall 0.67, arbor/arbour 0.91). The cutoff sits at 0.6,
between john/jane at 0.50 and every legitimate word difference above it.

Single-word names are exempt on purpose: with one token the whole-name floor
already IS the token check, and applying this on top would reject real variants
that have no long shared word to hide behind ("Nick"/"Nicolas" is 0.55).

**A decorated form of a stored name forked off it.** "Wren 🎵" against a stored
"Wren" has identical trigram sets — pg_trgm builds trigrams per word, so only the
separators differ — yet the score reads the sequence ratio (0.80 -> 0.40) and
needed history to reach 0.6. The in-batch pass already unifies such names on the
name alone and at a *lower* bar (0.5), so whether two forms were one entity
depended only on whether they arrived in the same retain; #3107 fixed that half
only. Identical trigram sets now reuse the candidate outright.

The candidate-cap test counted SequenceMatcher calls as a proxy for "candidates
scored"; the word-level check calls it too, and a candidate the trigram gate
rejects never reaches the name score, so it now counts the word-level check —
once per scored candidate, used nowhere else. The cap itself is unchanged.

Left as the one known limitation, now with the proof: Iran/Iraq is a single
character apart, and so are Alice/Alise (0.33 trigram) and Sara/Sarah (0.57),
which are real merges scoring below and around it. No character-level threshold
can order "different entity, similar spelling" above "same entity, different
spelling", and the word rule cannot help — both names are one word. That one
needs the extractor's type or an adjudication step.

* docs(entities): correct three comments the resolution change made stale

From code review of the branch, no behaviour change:

- _tokens_are_compatible lowercases its own input now. _trigram_set does, so a
  mixed-case caller would have silently disagreed with the gate beside it.
- The resolve=True contract said similar entities "are scored"; there is a floor
  in front of the score now, so say which.
- The max_candidates note said scoring is one SequenceMatcher call per
  candidate. It is a trigram set per candidate, plus a word-level check and a
  SequenceMatcher pass for those clearing the floor — which is also why the
  cap's own test no longer counts SequenceMatcher.
N
Nicolò Boschi committed
92695c6e5e34ed5fb4052422c6fc517817ba657e
Parent: b448202
Committed by GitHub <noreply@github.com> on 8/24/2026, 11:45:01 AM