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 detection —
apps/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 short —
NAME_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:
| Reason | Evidence | Starts pre-ticked? |
|---|---|---|
| Same email | Exact — near-certain same person | Yes, whole cluster |
| Same phone | Exact — near-certain same person | Yes, whole cluster |
| Similar name | A hint; two people can share a name | No — 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
- Name functions after the question they answer, not the shape of their
output.
computeNameClustersdescribes what it returns. Had it been calledsuggestCommunityTagClusters, the wrong call site would have read as obviously wrong to anyone skimming the file. - 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.
- 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.
- 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.
- 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.
- 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.
Discussion
The fan-out that killed the search
Six keyword sources under one Promise.all, each awaiting its own scoped tenant connection, against a pool of three. Search returned HTTP 500 with a single user on it. Why Promise.all is a concurrency multiplier, not a performance tool, when every read checks out a connection.
One token cannot be both
Light mode was audited to WCAG AA and the palette itself turned out to be the bug. On a light ground a colour's contrast as a fill and its contrast as text multiply to a fixed constant — ours is 16.78 — so no single value can clear 4.5:1 at both jobs. Why every accent in the system is now a pair.