Ann and Bob don't exist. They're two synthetic strangers in the test workspace of a CRM bridge I was about to publish. Ann is ann@x.example, Bob is bob@x.example, and both typed N/A into the phone field of a lead form, because that's what people type into a required phone field they'd rather not fill. My fake CRM merged them. Bob's upsert came back carrying Ann's contact id with Ann's conversation history hanging off it, and the audit ledger recorded contact_created twice. Two people created, one person stored, and the second creation line quietly naming the first one's id.
I caught it in a pre-publish review pass, not in production, which is the only comfortable part of this story. It's the third time in a few months I've hit the same family of bug, and this one stings in a specific way: it walked past a doctrine I had already written down twice.
The first incident was production. Python's casefold() merged two of my customers into one tenant: Unicode case folding is many-to-one, U+212A KELVIN SIGN folds to k, and two distinct identifiers became one entitlement key. The second was last week. My new repo argued against the exact bug it was carrying: the package preached ASCII-only folding while its own test fakes normalised emails with str.lower(). The doctrine existed; a caller just didn't call it.
This third one is worse in an instructive way, because the doctrine was right in both places you'd think to look.
The repo, for context
ghl-bridge is a policy-gated bridge between a GoHighLevel location and whatever generates reply drafts. Webhook in, dedupe, then a gate that only auto-sends a message when every policy passes, idempotency keyed on the event rather than the delivery, and a guard that raises if anything unapproved tries to leave. 379 tests, all offline, no account and no API key behind any of it.
Identity in this package follows two rules I've now paid for personally. Email folds ASCII case only, because str.lower() and str.casefold() apply Unicode tables that merge characters which were never case variants of each other. And phone is deterministic E.164 or an explicit refusal: a bare national number resolves only through the location's configured region, and when the rules can't determine the answer, the result is a PhoneNeedsReview value carrying the raw input and a named reason. A value, not an exception. The caller has to route it to a human queue on purpose, because guessing a country code merges strangers.
Both rules held. normalise_email was clean. normalise_phone was clean. Every caller I'd checked called them.
The door nobody was watching
The fake workspace models the platform's documented upsert semantics: within a location, an incoming contact that matches on email or phone updates the existing record instead of creating a new one. To do that it keeps an email index and a phone index, and before the review pass, its phone key helper had a fallback:
def _phone_key(self, state: _LocationState, raw: str) -> str:
result = normalise_phone(raw, default_region=state.location.default_region)
if isinstance(result, NormalisedPhone):
return result.e164
return raw.strip()
Look at how reasonable it is. The doctrine function is right there, called first. The fallback only fires for values the normaliser refused, and it just keeps them, tidied up, so the index stays total. Every visible key path in the package was doctrine-clean, and then the else branch of one private helper inside the test fake overruled the whole argument.
Run the sequence and the consequences arrive in order. Ann upserts with phone N/A; the normaliser refuses; the index files her under the literal string N/A. Bob arrives an hour later with the same placeholder; his email misses; his phone "matches". Merge. From then on, every lead whose phone field says N/A chains into that same contact record, which keeps getting fatter. Meanwhile the deduper, which computes its own doctrine-clean keys, saw nothing to match for Bob, so it logged contact_created while the store underneath merged him away. The audit trail and the data disagreed. In a real CRM this is one stranger reading another stranger's conversation history.
And the detail that made me laugh, the bitter kind: raw.strip() is the Unicode strip. The package's own identity.py defines a six-character ASCII whitespace constant precisely because bare str.strip() trims things like U+3000 IDEOGRAPHIC SPACE and merges identifiers that differ only there. So ext-12 and ext-12 followed by an ideographic space also folded onto one index key. The fallback didn't merely bypass the phone doctrine. It committed the exact sin from the email doctrine on its way past.
The fix is structural, and it's None
def _phone_key(self, state: _LocationState, raw: str) -> str | None:
"""The E.164 dedupe key, or None when the number does not determine
one. An unresolvable phone is stored as a field but never indexed:
indexing the raw string would merge two strangers who both typed
"N/A" into the form, and trimming it with ``str.strip`` would break
the ASCII-only doctrine everything else keys on. No key, no match,
no merge."""
result = normalise_phone(raw, default_region=state.location.default_region)
if isinstance(result, NormalisedPhone):
return result.e164
return None
The callers grew None checks: indexing skips it, phone search returns empty. The raw value still gets stored on the contact as a field, so a human reading the record sees exactly what arrived; it just never becomes a match key. Three tests now pin the behaviour: test_two_strangers_with_the_same_unparseable_phone_do_not_merge is the Ann and Bob story, test_an_unparseable_phone_is_stored_as_a_field_but_never_indexed pins the stored-but-never-keyed split, and test_the_ascii_trim_doctrine_holds_for_phone_indexing_too carries a literal U+3000 in its source. You can't see the character, which is exactly the point.
Two lessons worth carrying out of the repo, because both apply anywhere identity gets deduped.
A normalisation failure is not an identity. When the normaliser says "I don't know", indexing the raw value turns every placeholder into a master key: every N/A equals every other N/A, every none, every -, every xxx. This isn't a CRM quirk. Any dedupe, ETL join, or entity-resolution pass keyed on a column that contains placeholders has this failure mode, and the placeholder density of a phone column is never zero. The irresolvable key has to index as nothing at all.
One rule, and it's only as strong as its least visible door. Three incidents, one shape: a many-to-one transformation over identity is a merge, and a merge is a grant. In the tenant incident the grant was entitlements. In the on-behalf repo it would have been document access. Here it's two strangers becoming one contact with one shared history. What moved between the incidents is where the door was. In incident two, the doctrine existed and a caller didn't call it. In incident three, the callers all called it, and the leak was the fallback path inside one of them, in a test fake, the component everyone extends and nobody audits because "it's just the fake". The fake is the thing your entire suite treats as ground truth. A doctrine that stops at the fake's front door is a lobby sign.
What the fix does not fix
A placeholder that happens to parse walks straight through the front door: 5555555555 under a US-region location resolves to a legitimate-looking E.164 and mints a real key, so two strangers typing it will still merge. Refusing the unparseable stops junk from acting as a wildcard; it does not detect junk. Catching parseable junk needs a known-junk list or an alarm on keys that match suspiciously often, and this repo ships neither yet. Second, the fake is my model of the documented upsert semantics; whether the live platform's own server-side matching indexes raw strings is exactly the kind of thing the RUNBOOK says to verify against a real workspace before trusting anything. Third, the fix has a stated cost: an N/A lead with an unknown email now creates a fresh contact every time. That's deliberate. It fails toward duplicates a human can merge instead of merges nobody can split, but your review queue will feel the difference.
The doctrine, the fake, the deduper and the three anchor tests are all in github.com/vinimabreu/ghl-bridge if you want to run the Ann and Bob case yourself.
A key you couldn't compute is not a key to tidy up. It's no key at all.
Top comments (3)
This one is sharper than the last, and I think the reason is in a design choice you made on purpose and then paid for.
PhoneNeedsReviewis a value carrying the raw input and a named reason, and you say plainly why: the caller has to route it to a human queue deliberately. The instinct behind it is right. It also leaves the refusal representable as data, and anything representable as data can be turned back into an answer by any caller who would rather have one. Yourreturn raw.strip()performs that conversion in one line, in a private helper, inside a fake.So the doctrine really did hold in both places you would look. The normaliser refused correctly. What failed is that the refusal was a thing you could decline to honour, and declining looked like tidying.
We ran into the same shape on a price lookup and it went unnoticed for days. A
.get()on a backend-to-rate map fell through to a stale default, so a retired tier's price got attached to live requests. Nothing errored. A lookup that answers with a default is indistinguishable from one that answers, and the number it produced was plausible enough to sit in a report and get quoted. The rule we took out of it stayed narrow. An unfetchable rate has to be an error, never a fallback value.The fix that stuck was making the refusal unconvertible at the point of publication. Our benchmark harness now declines to print a cost at all when no rate row exists, and prints the reason where the number would have been. A caller cannot tidy that into a value, because there is no value to tidy.
Which suggests the question for a value-shaped refusal: who is allowed to observe it and still proceed? For
PhoneNeedsReviewthe answer should probably be the human queue and nothing else, and an index builder is not that. Worth grepping for every site that pattern-matches the good case and has an else branch, since the else branch is where the doctrine gets quietly overruled by something that reads as housekeeping.The part I keep taking from this series is that all three had a correct rule written down. None of them failed on the rule.
You put your finger on the exact line, and the damning part is whose hand was on it. return raw.strip() is in my fake. Nothing downstream coerced the refusal first, I did, in my own test double, the moment I wanted a value out of it. That's the tell: if a refusal can be turned back into an answer, the first caller who does it is the author, and it doesn't feel like overruling the rule, it feels like getting the string you were already holding.
The convertibility has a specific source: PhoneNeedsReview carries raw. The reason is doing honest work, but raw sitting next to it is a loaded value, answer-shaped, one strip() from live. A refusal that keeps a copy of the thing it refused is a sum type with one arm still holding the other arm's payload. The fix that matches your "who may observe and still proceed" is to make raw unreachable except through the human queue, a capability instead of a public field. Then the index builder can't strip it because it can't see it, and the discipline stops depending on nobody writing an else branch.
Your price lookup is the same move at the other end. Printing the reason where the number would go makes the refusal occupy the answer's slot without being answer-shaped, so the consumer that wanted a cost gets something it can't coerce. Unfetchable rate as an error, not a fallback, is the phone that doesn't index: both refuse to leave a valid-looking value where a real one belongs. The stale default and the strip() are one failure, a refusal quietly converted at the point where someone preferred an answer.
Which lines your two comments into a single rule. Last thread: a flag has no door. This one: a refusal shaped like a value has no door either. The doctrine lives in the normaliser, which is a function, a door. What it returns is data, and data gets re-read by whoever wants it read their way. So the job was never refusing correctly, both of these refused correctly. It's keeping the refusal doored all the way to publication, which is your unconvertible-at-the-edge. All three failed one level under the rule, in the type the rule hands back.
The capability framing is a good end state, and there is one level below it that we paid for after we had already built the doored version.
We struck a figure at the claim. The ledger row was marked, the harness refused to print it, and the number became correctly unavailable everywhere the rule ran. It then turned up in a second document that had quoted it weeks earlier, sitting in a sentence, carrying no marker and pointing at nothing.
The refusal was doored all the way to publication and undoored one step past it, because publication was more than one door. A derived value leaves the store and starts living in prose, and prose has no type.
What that changed for us was small and specific. A published number has to carry enough with it to be re-derived, and something has to refuse a superseded figure quoted as current. Both of those are checks on the copies, since in every case that hurt us the source was already correct.
So the capability gets you the part a compiler can hold, which is most of the value and worth doing. What it leaves standing is the sentence somebody wrote while the value was still valid. That one wants a different instrument, and ours is newer and weaker than the normaliser it sits behind.