dhaga.blog
Engineering

The matcher that meant something else

Duplicate detection collapsed unrelated people into one contact. The helper it called was not buggy — it was a community-tag suggester that groups by surname on purpose, and it is still shipping unchanged. Why the types could never have caught this, and why making the shared helper smarter was the wrong fix.

The short version

Dhaga's duplicate-contact page offered to merge people who had nothing in common but a surname. On a contact list where dozens of people share one family name, it proposed folding all of them into a single record — and the merge button was one click.

The obvious hypothesis is that the matching function had a bug. It did not. The function it called, computeNameClusters(), deliberately drops the given name and groups on the surname, because it was written to suggest community tags at import time, where a shared family name is exactly the signal you want. It was correct for its own caller then, and it is still correct and still shipping, unchanged.

The defect was at the call site. Duplicate detection asks "is this the same person?"; the helper answers "do these people share a community?". Both take a list of names and return groups of names, so nothing in the type signature could object. The fix was to give duplicate detection its own key function — fullNameKey(), which requires every token of the name to agree — and to leave the shared helper alone.

The rest of this post is the deep dive: why a correct function produced a catastrophic result, the tempting fix we rejected because it would have broken the caller the helper was right for, what the whole-name key actually does, and what changed in the UI once we admitted a name match is only a hint. File paths refer to the real code.


Two questions that look like one function

Dhaga has two features that both start from a pile of contact names.

The first is a community-tag suggester. Contacts arrive from CSV exports and card scans with no structure at all, and people encode context in names. If twenty of your contacts share a family name, "tag all of these" is a genuinely useful offer. That lives in apps/web/src/lib/suggestions/name-clusters.ts, and its real caller is getSuggestedClusters() in apps/web/src/lib/repo/suggestions.ts. It is confirm-only: a cluster becomes a tag when the user says so, never before.

The second is duplicate detectionapps/web/src/lib/repo/contacts/duplicates.ts. It reads every contact and groups the ones that look like the same person, so you can merge them. Merging is destructive: fourteen tables get repointed, and the source contact rows are hard-deleted at the end of the same transaction.

Both features want to group contacts by name. So the second one called the first one. That is the entire bug.

The helper is not wrong

Here is the line that decides everything, in name-clusters.ts:

// name-clusters.ts — correct, and still shipping unchanged.
// The first space-separated word is the given name — clustering on it
// ("all Amits") is noise, so it's excluded whole, symbols and all.
const words = contact.name.trim().split(/\s+/).slice(1);

.slice(1) throws away the given name on purpose. For tag suggestions that is the whole point — the given name is the part that varies, and the shared remainder is the community you might want to label. The comment above it says so. It has said so since the function was written.

Read as a duplicate detector, the same line says: ignore the part of the name that distinguishes these people, and group them by the part they have in common. For a surname shared by millions of people, that means every one of them reads as the same human being.

There was a second, stranger consequence. The clusterer skips a token a contact already carries as a tag or a company name, because there is nothing left to suggest:

if (contact.tags.includes(key)) continue;
if (contact.companyName?.toLowerCase() === key) continue;

Perfectly reasonable for a suggestion engine. Through the shared call, it meant which contacts appeared as duplicates depended on which tags you had already confirmed. Accepting a tag suggestion quietly changed the duplicates page. Nobody designed that. It fell out of one function serving two questions.

Why the types were never going to save us

This is the part worth generalising.

computeNameClusters(contacts, minSize, limit) takes a list of records carrying a name and returns groups of contact ids. Duplicate detection had a list of records carrying a name and wanted groups of contact ids. It built the input array, called the function, and compiled cleanly on the first try.

The function's name describes its shape — cluster these names — not its meaning: group people who are plausibly the same family. strict: true buys you nothing here. There is no type that distinguishes "grouped because they are one person" from "grouped because they share a community", because both are string[] of ids.

The types did carry one hint, and we missed it. The input type ClusterableContact requires tags and companyName — fields a duplicate finder has no business caring about. Duplicate detection dutifully populated them to satisfy the signature. A parameter you have to fabricate a value for is a signal the function was written for someone else. It is a soft signal, and it compiles either way.

The fix we rejected

The tempting fix is to teach the shared helper the difference. Add a mode: "duplicates" | "tags" flag, or a requireFullName boolean, and branch on it inside computeNameClusters().

We rejected it, and the reasoning matters more than the outcome.

The helper is not broken, so there is nothing to fix inside it. A flag would put two incompatible definitions of "these names go together" in one function, where every future change has to be checked against both. And the failure mode is asymmetric: get the tag branch subtly wrong and you show a bad suggestion the user declines. Get the duplicate branch wrong and you merge two strangers into one contact — a destructive write across fourteen tables, on data the user cannot reconstruct.

There is also a smaller point that decided it. A flag makes the wrong behaviour reachable from the wrong caller forever after. Every new call site now has to know which mode it wants, and the default — whichever one you pick — will be wrong for somebody.

Two questions, two functions.

What shipped

Duplicate detection now owns fullNameKey(), in apps/web/src/lib/suggestions/full-name-key.ts. Every token of the name is normalised, honorifics are dropped, the tokens are sorted, and the result is joined into a key. Same-key contacts group; everyone else stays apart.

// Before. duplicates.ts borrowed the tag suggester.
clusterable.push({ id: row.id, name: row.name, tags: row.tags, companyName: row.companyName });
// ...
for (const cluster of computeNameClusters(clusterable, 2, DUPLICATE_CLUSTER_LIMIT)) {
  clusters.push({ reason: "name", contacts: toItems(cluster.contactIds) });
}
// After. duplicates.ts keys on the whole name itself.
const nameKey = fullNameKey(row.name);
if (nameKey) addTo(byName, nameKey, row.id);
// ...
for (const [key, ids] of byName) push(key, "name", ids);

Three decisions inside fullNameKey() are load-bearing:

Tokens are sorted. Name order varies by source — a CSV writes "Singh, Amit" where a card scan writes "Amit Singh". Sorting makes those one key while keeping two different people with the same surname apart, which was the entire point.

A single-token name returns null. A lone given name is not evidence that two records are one person. Grouping every contact called "Amit" is the original bug reproduced one first name at a time, so the function refuses to key at all.

The honorific list is deliberately shortNAME_HONORIFIC_TOKENS in apps/web/src/utils/constants/people.ts holds nine entries. A longer list starts eating real name tokens, and a false strip merges two different people. A missed match costs the user a manual merge; a false match costs them a person.

The tests in apps/web/src/lib/__tests__/contact-duplicate-name-key.test.ts encode the argument rather than the mechanics — the first one is named "keeps people who merely share a surname apart — 'Singh' is a community, not a person", and asserts that Amit Singh, Ravi Singh and Priya Singh produce three distinct keys.

name-clusters.ts was not touched.

A name match is a hint, not a verdict

Fixing the key was only half of it. Even a perfect whole-name match is weaker evidence than an exact shared email, because two people really can have the same name. The page now says so in its behaviour:

ReasonEvidenceStarts pre-ticked?
Same emailExact — near-certain same personYes, whole cluster
Same phoneExact — near-certain same personYes, whole cluster
Similar nameA hint; two people can share a nameNo — nothing selected

Sections render strongest-first, and every person in a cluster carries a checkbox, so five people who look alike can become one merge of the three that actually are. That rule lives in DUPLICATE_CONTACT_EXACT_REASONS, with the comment "Pre-ticking a fuzzy match is how you merge two strangers."

The same pass also stopped emitting one pair twice when it matched on two signals. The de-duplication keys on the sorted set of contact ids, so it is exactly a repeat of the same people that gets dropped, under the strongest reason that produced it. A wider name cluster overlapping a narrower email one is a different set, so both still show — which is right: they are different claims about different people. The pass also made the merge action revalidate the duplicates page, which it never had, so the page could previously offer you a contact the merge had already deleted.

The takeaways

  1. Name functions after the question they answer, not the shape of their output. computeNameClusters describes what it returns. Had it been called suggestCommunityTagClusters, the wrong call site would have read as obviously wrong to anyone skimming the file.
  2. Matching types are not matching semantics. Two callers can pass the same input type and receive the same output type while asking incompatible questions. A type system cannot see the difference, and a strict one will give you no warning at all.
  3. A parameter you have to invent a value for means the function was written for someone else. Populating fields your feature does not care about is the cheapest available smell — notice it before it compiles.
  4. Do not add a mode flag to a correct function. It puts two definitions in one body, makes the wrong behaviour reachable from every future caller, and the safer branch is the one that pays for the other's mistakes.
  5. Weigh reuse by what a wrong answer costs. Sharing a helper between a suggestion (declined in one tap) and a destructive merge (unrecoverable) is a bad trade even when the helper is flawless.
  6. When evidence is weak, make the UI say so. An exact match may arrive pre-selected; a fuzzy one should require the user to affirm each row.
Share

Discussion

On this page