Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
933bc86
feat(duplicates): derive clusters with dismissed pairs as negative edges
bashar-qassis Aug 18, 2026
07286ca
fix(duplicates): tenancy-scope cluster contact lookup, fix vacuous de…
bashar-qassis Aug 18, 2026
b422e33
feat(duplicates): add cluster lookup, primary selection and clique di…
bashar-qassis Aug 18, 2026
8c6e29b
fix(duplicates): cover merged-preservation branch, transactional cliq…
bashar-qassis Aug 18, 2026
c838223
feat(contacts): summarise multi-valued records and history for a merge
bashar-qassis Aug 18, 2026
24ff382
fix(contacts): guard blank-key addresses in merge summary from false-…
bashar-qassis Aug 18, 2026
8ce0a51
feat(contacts): expose candidate values for any mergeable field
bashar-qassis Aug 18, 2026
7d58058
fix(contacts): break candidates_for/2 ties on lowest member id
bashar-qassis Aug 18, 2026
7e1c0d6
feat(duplicates): list clusters instead of pairs
bashar-qassis Aug 18, 2026
f6b2361
perf(duplicates): index dismissed pairs and derive clusters once per …
bashar-qassis Aug 18, 2026
39d2097
feat(duplicates): render the cluster merge screen
bashar-qassis Aug 18, 2026
027c6c1
fix(duplicates): honor Leave empty carve-out, resolved-state badge, f…
bashar-qassis Aug 18, 2026
172bc96
feat(duplicates): wire cluster merge screen interactions
bashar-qassis Aug 18, 2026
9fa0692
fix(duplicates): pin C5 discard, cover toggle-value/set-primary, guar…
bashar-qassis Aug 18, 2026
41aaa2c
feat(duplicates): submit cluster merges and rejections
bashar-qassis Aug 18, 2026
6391a7c
fix(duplicates): cover alias drops, align address dedupe key with the…
bashar-qassis Aug 18, 2026
bf33044
feat(duplicates): render immich links as one editable row
bashar-qassis Aug 18, 2026
5643986
fix(duplicates): guard immich choice against deselected members, fix …
bashar-qassis Aug 18, 2026
f8e9c55
fix(duplicates): make resolved rows editable, show policy flags, guar…
bashar-qassis Aug 18, 2026
9f10da0
fix(duplicates): treat not-null columns as non-clearable, keep identi…
bashar-qassis Aug 19, 2026
31026a3
fix(contacts): declare t/0 on the schemas Cluster's typespec references
bashar-qassis Aug 19, 2026
904e784
Merge remote-tracking branch 'origin/worktree-merge-revamp' into merg…
bashar-qassis Aug 27, 2026
9e4bd4b
test: align cluster merge field coverage with coupled year-unknown flags
bashar-qassis Aug 27, 2026
985acdf
fix: close the dismissal race and validate crafted merge-screen payloads
bashar-qassis Aug 27, 2026
f4aa6f8
perf: bound the cluster derivation and cache the duplicates badge
bashar-qassis Aug 27, 2026
68ebb90
test: stop the ToS flag leaking between concurrent async cases
bashar-qassis Aug 27, 2026
5343a52
docs: state merge-screen comment rationale without narrating code his…
bashar-qassis Aug 27, 2026
da29e67
fix: keep merge-screen spec comments out of the rendered HTML
bashar-qassis Aug 28, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions lib/kith/contacts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,47 @@ defmodule Kith.Contacts do
Merge.run(scope, survivor_id, loser_ids, resolution)
end

@doc """
Names for the association ids `members` hold, as `%{field => %{id => name}}`.

Design spec §3, "Association ids": a merge screen renders the associated
record's name, never the raw id. Covers only the three association columns
in `Kith.Contacts.MergeFields.choice_fields/0`; ids with no surviving row
are simply absent, and the caller falls back to the id.

`first_met_through_id` may point at any contact in the account, not only a
cluster member, so it is looked up rather than read off `members`.
"""
def merge_association_labels([]), do: empty_association_labels()

def merge_association_labels([%Contact{} | _] = members) do
account_id = hd(members).account_id

%{
gender_id: label_map(Gender, ids_held(members, :gender_id), :name),
currency_id: label_map(Currency, ids_held(members, :currency_id), :name),
first_met_through_id:
Contact
|> where([c], c.account_id == ^account_id)
|> label_map(ids_held(members, :first_met_through_id), :display_name)
}
end

defp empty_association_labels,
do: %{gender_id: %{}, currency_id: %{}, first_met_through_id: %{}}

defp ids_held(members, field) do
members |> Enum.map(&Map.fetch!(&1, field)) |> Enum.reject(&is_nil/1) |> Enum.uniq()
end

defp label_map(_queryable, [], _name_field), do: %{}

defp label_map(queryable, ids, name_field) do
from(r in queryable, where: r.id in ^ids, select: {r.id, field(r, ^name_field)})
|> Repo.all()
|> Map.new()
end

@doc """
Merges `non_survivor_id` into `survivor_id`.

Expand Down
2 changes: 2 additions & 0 deletions lib/kith/contacts/contact.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ defmodule Kith.Contacts.Contact do
default_limit: 20,
max_limit: 100}

@type t :: %__MODULE__{}

schema "contacts" do
field :first_name, :string
field :last_name, :string
Expand Down
2 changes: 2 additions & 0 deletions lib/kith/contacts/duplicate_candidate.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ defmodule Kith.Contacts.DuplicateCandidate do

@statuses ~w(pending merged dismissed)

@type t :: %__MODULE__{}

schema "duplicate_candidates" do
field :score, :float
field :reasons, {:array, :string}, default: []
Expand Down
4 changes: 4 additions & 0 deletions lib/kith/contacts/merge.ex
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,10 @@ defmodule Kith.Contacts.Merge do
# case/whitespace-only differences still collapse. Tags and photos
# are deduped by their own remap steps above already, via
# `dedupe_and_move/6`; nothing to repeat here.
#
# `Kith.Contacts.MergeSummary.normalize/1` mirrors this key so the screen
# marks exactly the rows this step collapses. `btrim/1` strips spaces only —
# keep the two in step if either side changes.
defp dedupe_owned_step(repo, survivor_id) do
repo.query!(
"""
Expand Down
84 changes: 79 additions & 5 deletions lib/kith/contacts/merge_fields.ex
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,87 @@ defmodule Kith.Contacts.MergeFields do
def known?(field), do: field in all()

@doc """
Fields that cannot be set to `:clear`, derived from the contact update
changeset's `validate_required/2` list rather than maintained by hand.
Fields that cannot be set to `:clear`.

Two sources, neither maintained by hand:

* the contact update changeset's `validate_required/2` list, and
* every `contacts` column declared `NOT NULL`.

The second half is not optional. `:clear` becomes `nil` at the database, so a
`NOT NULL` column raises a `Postgrex.Error` (SQLSTATE 23502) outside the
merge engine's documented `{:error, reason}` contract — and the changeset's
required list does not cover them: `birthdate_year_unknown` and
`first_met_year_unknown` are `null: false` booleans that no
`validate_required/2` names, so the column check is the only thing standing
between them and a crash on the cluster merge screen.

Read from `information_schema` rather than restated here so a column added
with `null: false` later is covered on its own.
"""
def non_clearable do
%Contact{}
|> Contact.update_changeset(%{})
|> Map.fetch!(:required)
cached(:non_clearable, fn ->
changeset_required =
%Contact{}
|> Contact.update_changeset(%{})
|> Map.fetch!(:required)

Enum.uniq(changeset_required ++ not_null_fields())
end)
end

@doc """
Mergeable fields backed by a `NOT NULL` column on `contacts`.
"""
def not_null_fields do
cached(:not_null_fields, &query_not_null_fields/0)
end

# Both halves of `non_clearable/0` are cached, not just the query: the
# changeset half builds a `%Contact{}` and runs `update_changeset/2` over it,
# and `non_clearable?/1` is called once per choice field per render — 17
# changesets a render — and again for every field inside the merge
# transaction.
#
# `:persistent_term` because both answers are properties of the deployed
# schema and the compiled changeset, so they cannot change while the node is
# running. A migration that relaxes a nullability constraint is picked up on
# the next restart, which is when the migration's own deploy happens anyway;
# there is no purge path because nothing would call it.
defp cached(name, fun) do
key = {__MODULE__, name}

case :persistent_term.get(key, nil) do
nil ->
value = fun.()
:persistent_term.put(key, value)
value

value ->
value
end
end

# Compared as strings and filtered against the registry, so no atom is
# created from a database identifier.
#
# Scoped to `current_schema()`: `information_schema.columns` spans every
# schema in the database, so without it any other `contacts` table — a
# leftover, a second tenant schema — contributes its `NOT NULL` columns to
# the union, and a column nullable here but `NOT NULL` there would be
# reported non-clearable and hide the "Leave empty" button.
defp query_not_null_fields do
%{rows: rows} =
Kith.Repo.query!("""
SELECT column_name FROM information_schema.columns
WHERE table_name = 'contacts'
AND table_schema = current_schema()
AND is_nullable = 'NO'
""")

names = rows |> List.flatten() |> MapSet.new()

Enum.filter(all(), &MapSet.member?(names, Atom.to_string(&1)))
end

@doc "Whether `field` may be cleared."
Expand Down
16 changes: 16 additions & 0 deletions lib/kith/contacts/merge_resolution.ex
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,22 @@ defmodule Kith.Contacts.MergeResolution do
end
end

@doc """
Every distinct value `members` hold for `field`, most-held first, ties broken
by the lowest member id holding that value. Callers may rely on this order
being stable across renders.

Unlike `conflicts`, this is populated even when the members agree — the screen
uses it to open an already-resolved row as a choice.
"""
def candidates_for(members, field) do
members
|> Enum.map(fn member -> {member.id, normalize(Map.fetch!(member, field))} end)
|> Enum.reject(fn {_id, value} -> is_nil(value) end)
|> candidates()
|> Enum.sort_by(&{-&1.count, Enum.min(&1.member_ids)})
end

# Returns {resolved_value, attribution, conflict_candidates}
defp resolve_scalar(members, field) do
held =
Expand Down
198 changes: 198 additions & 0 deletions lib/kith/contacts/merge_summary.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
defmodule Kith.Contacts.MergeSummary do
@moduledoc """
Gathers the multi-valued records and history counts a merge screen shows.

Read-only. Duplicates are marked rather than removed so the screen can show
what the merge will collapse instead of silently dropping it.
"""

import Ecto.Query, warn: false

alias Kith.Contacts.{Address, ContactField, Document, Note, Photo}
alias Kith.Repo

@history [
notes: Note,
documents: Document,
photos: Photo,
calls: Kith.Activities.Call,
life_events: Kith.Activities.LifeEvent,
reminders: Kith.Reminders.Reminder,
gifts: Kith.Contacts.Gift,
debts: Kith.Contacts.Debt,
pets: Kith.Contacts.Pet,
tasks: Kith.Tasks.Task,
conversations: Kith.Conversations.Conversation,
relationships: Kith.Contacts.Relationship
]

@doc """
Builds the summary for `members`.

Every `contact_fields`/`addresses`/`tags`/`aliases` entry also carries a
`key` — the value `mark_duplicates/1` grouped it by to decide `duplicate?`.
This is not part of the documented entry shape but is kept intentionally:
`Kith.Contacts.Merge`'s `:dedupe_owned` step runs before `:apply_drop`, so a
survivor row and a loser row with an equal, normalized value are
equivalent — dropping only the one the user clicked can leave its
equivalent behind and the value comes back. Callers building a drop list
must expand a dropped entry to every entry sharing its `key`, not just the
one row shown as a duplicate.
"""
def build(members) do
ids = Enum.map(members, & &1.id)

%{
contact_fields: contact_fields(ids),
addresses: addresses(ids),
tags: tags(ids),
aliases: aliases(members),
history: history(ids)
}
end

defp contact_fields(ids) do
from(f in ContactField,
where: f.contact_id in ^ids,
join: t in assoc(f, :contact_field_type),
order_by: [asc: f.id],
select: {f, t.name}
)
|> Repo.all()
|> Enum.map(fn {field, name} ->
%{
id: field.id,
label: name,
value: field.value,
owner_id: field.contact_id,
key: {field.contact_field_type_id, normalize(field.value)}
}
end)
|> mark_duplicates()
end

defp addresses(ids) do
from(a in Address, where: a.contact_id in ^ids, order_by: [asc: a.id])
|> Repo.all()
|> Enum.map(fn address ->
%{
id: address.id,
label: address.label || "Address",
value: Enum.join(Enum.reject([address.line1, address.city], &is_nil/1), ", "),
owner_id: address.contact_id,
key: address_key(address)
}
end)
|> mark_duplicates()
end

# Mirrors the blank-key guard in Kith.Contacts.Merge's dedupe_owned_step/2:
# an address with neither line1 nor postal_code isn't a duplicate of
# another address in the same shape (they may differ by city/region/
# country, e.g. "Paris" vs "Tokyo"), so give each a key that can't collide.
#
# The engine compares these columns through `coalesce(col, '')`, so a NULL
# and an empty string are the same value to it. Collapsing both to `""` here
# keeps this key in step: a caller expanding a dropped entry to everything
# sharing its key (see `build/1`) would otherwise miss the other side of a
# nil/"" pair the engine is about to dedupe, and the dropped value would come
# back on the survivor.
defp address_key(address) do
line1 = blank_to_empty(normalize(address.line1))
postal_code = blank_to_empty(normalize(address.postal_code))

if line1 == "" and postal_code == "" do
{:no_key, address.id}
else
{line1, postal_code}
end
end

defp blank_to_empty(nil), do: ""
defp blank_to_empty(value), do: value

defp tags(ids) do
from(ct in "contact_tags",
join: t in Kith.Contacts.Tag,
on: t.id == ct.tag_id,
where: ct.contact_id in ^ids,
order_by: [asc: t.name],
select: {ct.contact_id, t.id, t.name}
)
|> Repo.all()
|> Enum.map(fn {owner_id, tag_id, name} ->
%{id: tag_id, label: "Tag", value: name, owner_id: owner_id, key: tag_id}
end)
|> mark_duplicates()
end

# Aliases are unioned into a single set, unlike contact_fields/addresses/tags
# (which keep duplicate rows, marked, since each row is independently
# droppable). An alias array has no per-row identity to drop, so repeats
# across members are folded away here rather than surfaced as duplicate?.
defp aliases(members) do
members
|> Enum.flat_map(fn member ->
Enum.map(
member.aliases || [],
&%{id: &1, label: "Alias", value: &1, owner_id: member.id, key: &1}
)
end)
|> mark_duplicates()
|> Enum.reject(& &1.duplicate?)
end

defp history(ids) do
counts =
Map.new(@history, fn
# Relationships are the one history table a member can be on either end
# of, and `Merge`'s `remap_relationships/4` rewrites both `contact_id`
# and `related_contact_id`. Counting only the first endpoint reports 0
# for a member that is purely the *target* of relationships, which
# understates what the merge moves. A row with both endpoints in the
# cluster is still one row, so `or` counts it once.
{:relationships, schema} ->
{:relationships,
Repo.aggregate(
from(r in schema, where: r.contact_id in ^ids or r.related_contact_id in ^ids),
:count
)}

{key, schema} ->
{key, Repo.aggregate(from(r in schema, where: r.contact_id in ^ids), :count)}
end)

activities =
from(ac in "activity_contacts", where: ac.contact_id in ^ids, select: count())
|> Repo.one()

Map.put(counts, :activities, activities)
end

# The first occurrence of a key wins; later ones are what the merge collapses.
# `key` is kept on the returned entry — see the moduledoc on `build/1`.
defp mark_duplicates(entries) do
{marked, _seen} =
Enum.map_reduce(entries, MapSet.new(), fn entry, seen ->
duplicate? = MapSet.member?(seen, entry.key)
{Map.put(entry, :duplicate?, duplicate?), MapSet.put(seen, entry.key)}
end)

marked
end

# Pinned to the SQL in `Kith.Contacts.Merge`'s `dedupe_owned_step/2`, which
# keys on `lower(btrim(col))`. One-argument `btrim` strips SPACES ONLY, while
# `String.trim/1` strips every Unicode whitespace character — so a value with
# a leading tab would read as "duplicate · dropped" on screen while the engine
# kept both rows, and a caller expanding a dropped entry over everything
# sharing its key (see `build/1`) would then drop a value the user never
# excluded. `String.trim/2` with `" "` is byte-for-byte what `btrim` does.
#
# `lower()` vs `String.downcase/1` still differ for locale-specific casing
# (Postgres folds per the database collation; Elixir folds per Unicode
# default). Both are identical over ASCII, which is what these keys are in
# practice; the whitespace half is the one that had a reachable divergence.
defp normalize(nil), do: nil
defp normalize(value) when is_binary(value), do: value |> String.trim(" ") |> String.downcase()
end
Loading