Consolidate duplicate Person records and fix same-name member collisions

[jonfroehlich]

Consolidate duplicate Person records and fix same-name member collisions

Related: #1206 (live 500 on same-name members), #582 (2019 cleanup discussion — policy now superseded)

Summary

Over ~15 years we've accumulated many duplicate Person records. "Duplicate" actually covers two different problems that need opposite fixes, and conflating them is why this has never been cleaned up:

1. True duplicates — one human with multiple Person rows (created by un-deduped imports / hand entry over the years). These should be merged into a single canonical record, reassigning all related objects.
2. Genuine namesakes — two different people who share a name. These must stay separate, but they currently break the site: two members with the same derived url_name cause Person.MultipleObjectsReturned → HTTP 500 on /member/<url_name>/ (this is We should figure out how to support two people with the same name as members of Makeability Lab #1206; the live example was two jasminezhang rows).

This issue covers both: a safe, reviewable merge workflow for (1), and a de-collision + view-hardening fix for (2).

Background & root cause

• Person.url_name is auto-derived in Person.save() from first_name + last_name (lowercased, accent-folded, punctuation-stripped), with a numeric-suffix collision loop (jonfroehlich, jonfroehlich2, …).
• That collision loop is recent and only protects rows created/re-saved after it landed. Historical colliding rows predate it and have never been re-saved, so they still share a bare url_name (and some never-re-saved rows may still hold the model default 'placeholder'). This is why We should figure out how to support two people with the same name as members of Makeability Lab #1206 still fires despite the loop existing in the model today.
• The member view "fix" noted in our codebase audit — switching get_object_or_404 to filter(...).order_by('-bio_datetime_modified').first() — masks the 500 but does not solve namesakes: for two distinct people it silently returns one and makes the other's page permanently unreachable. The correct fix is unique url_names, not picking a winner.

⚠️ Operating model / access constraints (read before designing)

The maintainer can SSH into prod and read files, but cannot run docker/management commands directly on prod and cannot do privileged file ops (much of the tree is apache:makelab-owned; those go through CSE IT). This constrains how anything runs against prod data:

• Prod DB: PostgreSQL on grabthar.cs.washington.edu, reachable only via the recycle.cs.washington.edu jump host. Credentials live in config.ini on the prod server (not in git; mounted as a volume). A DBeaver/ssh -L tunnel through recycle is the intended direct-access path.
• Deploys: push to master → auto-deploys test; push a tag (e.g. 2.1.0) → deploys production. Build logs at /logs/buildlog.txt on each server.
• Migrations are gitignored and regenerated per-environment, so data migrations (RunPython) are unreliable here and must NOT be used. All data operations ship as management commands (the established pattern — see generate_slugs_for_old_news_items, remove_year_from_forum_name, etc.).
• Therefore: do all analysis and the merge against a local Django pointed at the prod DB through the tunnel (preferred — output stays off the server), with the entrypoint-wired one-shot as a fallback if the tunnel is rejected by pg_hba. Anything written to MEDIA_ROOT on prod (/cse/web/research/makelab/www/media) is web-served — never write CSVs of personal data there (the stale public dumped_data.json is exactly this mistake).

The merge surface (do not hardcode — enumerate)

Merging duplicate B → canonical A must relocate every relation pointing at B. Known relations involving Person:

Relation	Type	Notes
Publication.authors	M2M (sorted, sort_value)	Author order matters (citations/BibTeX). Preserve order; dedup if A & B both on a pub.
Talk.authors	M2M (sorted)	Same care.
Poster.authors	M2M (sorted)	Same care. (All three are Artifact subclasses; see fix_sortedm2m_columns.py.)
Video.authors	M2M	Confirm related name / whether sorted.
Position.person	FK
ProjectRole.person	FK
News.author	FK (nullable)
Position.advisor	FK → Person	Easy to miss — B may be another person's advisor.
Position.co_advisor	FK → Person	Same.
Position.grad_mentor	FK → Person	Same.

Implementation requirement: walk relations generically via Person._meta.get_fields() (handle ManyToManyRel, ManyToOneRel, and the self-referential FKs) rather than a hardcoded list, so schema drift can't silently orphan data. For sorted M2Ms, preserve sort_value ordering.

Also on merge:

• Scalar fields: fill A's blank fields from B (email, websites, bio, etc.); never overwrite a populated field on A.
• Images: every Person gets a random Star Wars default image in save(), so "has image" is meaningless — detect the default by path and only treat a non-default upload as real. Note Person has a pre_delete signal that deletes its image file; ensure deleting B doesn't remove a file A now relies on (filenames are per-person, but verify).

Proposed approach

Phase A — Fix #1206 (independent, ship first via normal deploy)

A1. Recompute url_name for all rows so the existing collision logic de-collides historical duplicates. New management command (e.g. recompute_url_names) that re-derives and saves; idempotent. Optional enhancement per #1206: prefer a middle-initial differentiator for namesakes (jasminexzhang / jasminelzhang) over a bare 2 suffix, for readable, stable URLs. A2. Harden website/views/member.py to resolve cleanly (404, not 500) if a collision ever recurs, instead of relying on .first(). A3. Regression test: /member/<url_name>/ returns 200 for each of two same-base-name people once de-collided (DB-touching test via DatabaseTestCase).

Phase A stops the active 500s and does not depend on the merge work below.

Phase B — Dedup cleanup (review-gated)

B1. Export command — export_people_for_dedup (read-only, CSV). One row per Person. Columns:

• Identity: id, first_name, middle_name, last_name, url_name, email, personal_website, github, linkedin
• Relation counts: pub_count, talk_count, poster_count, video_count, position_count, projectrole_count, news_authored_count
• Advisor refs (block deletion): advisor_count, co_advisor_count, grad_mentor_count (i.e. Position.objects.filter(advisor=p) etc.)
• total_refs = sum of all the above (0 ⇒ safe-to-delete shell)
• Disambiguation aids: has_real_image (non-default, detect by path), earliest_position_date, latest_position_date

Strictly read-only (no .save()). Run locally via the tunnel; write CSV off-server.

B2. Local analysis (human-in-the-loop). Cluster the CSV by accent-folded normalized (first, last) (don't rely on url_name). For each multi-row cluster, classify:

• Auto / safe: total_refs == 0 shells → delete (nothing to reassign; equivalent to merge). Also high-confidence merges where one row is a shell and another is rich, or emails match exactly.
• Review: same name, overlapping co-authors/projects/active-dates but no email match → likely same human, confirm by hand.
• Keep separate: disjoint emails / disjoint pubs / different middle names → namesakes (Phase A gives them distinct URLs).
• Cross-name same-person (e.g. the documented "Ji Hyuk Bae" = "Sean Bae" high-school→undergrad case): clustering can't find these; the decisions file must accept manually added source→target pairs.

Output: a reviewed decisions file (CSV/JSON) of explicit actions: merge into:<id> / keep / delete.

B3. Merge command — merge_duplicate_people (decisions-file-driven).

• Reads the committed/reviewed decisions file; --dry-run prints the full plan and changes nothing (default to dry-run).
• Each merge runs in transaction.atomic().
• Generic relation walk (above); preserve sorted-M2M order; scalar backfill; image handling.
• Idempotent and loud-logging (verification is via /logs/ over SSH, not a shell): after a merge the source id is gone, so re-runs are no-ops.
• Run locally against the prod DB via the tunnel (preferred), or entrypoint-wired one-shot fallback, then removed in a follow-up commit.

Acceptance criteria

• /member/jasminezhang/ (and any other historical collision) returns 200, with each distinct person reachable at a unique URL.
• export_people_for_dedup produces the CSV above and performs zero writes.
• merge_duplicate_people --dry-run reports the plan with no DB changes.
• A real merge reassigns all relation types in the table above, including the three advisor self-refs, and preserves publication author order.
• Merging is idempotent (safe to re-run).
• manage.py test website passes, including new regression tests:
  • Phase A: two same-base-name people both resolve (200).
  • Merge preserves Publication.authors order.
  • Merge reassigns FK relations (Position/ProjectRole/News) and advisor self-refs.
  • --dry-run makes no changes; second run of an applied merge is a no-op.

Use the existing DatabaseTestCase helpers (make_person, make_publication, make_news_item) per CONTRIBUTING.md.

Out of scope / follow-ups

• Prevention at the creation point (dedup check wherever Person rows are bulk-created/imported) — the real long-term fix so this doesn't re-accrue. Separate issue.
• Bulk deletion of orphaned person-image files left after merges (existing delete_unused_files / thumbnail_cleanup commands should cover this).

Open questions

1. Confirm the exact redeploy/restart trigger and one-shot conventions against CLAUDE.md + docker-entrypoint.sh before relying on the entrypoint fallback.
2. Confirm Video.authors related name and whether it's sorted.
3. For Phase A1, do we want the middle-initial differentiator now, or just de-collide with the existing numeric-suffix logic and defer readability?

[jonfroehlich]

Correction to the "Operating model / access constraints" section — verified 2026-06-14

While scoping the access path for this work we checked the actual prod/test setup, and the Operating model section above is inaccurate in ways that change the plan:

1. The database is the in-stack db Docker container, not an external Postgres. Prod/test config.ini contains only a [Django] section (no [Postgres]), so settings.py falls through to its default DATABASES (HOST='db') — the db service in docker-compose.yml. "grabthar" is just the host machine that runs the Docker stack, not a separately managed Postgres server.
2. DB credentials do not live in config.ini (the section claimed they did).
3. There is no direct DB path — no tunnel. The maintainer can SSH only to the recycle jump host (read-mostly file access on the shared filesystem); there is no shell access to the Docker host, and the DB port is bound to the host's loopback only, so it is unreachable from a laptop or from recycle. The DBeaver/ssh -L "intended direct-access path" described above does not exist.

Consequence: the "preferred" approach (local Django → prod DB via tunnel) is not possible. All data operations must run inside the prod container as management commands wired into docker-entrypoint.sh (the established one-shot pattern, verified via logs). For Phase B's offline analysis we will request a one-time read-only DB snapshot from CSE IT, do the clustering + merge testing locally, then ship the reviewed merge as an entrypoint one-shot.

The phased plan (A: de-collide url_names + harden the member view; B: snapshot → review → idempotent merge_duplicate_people) is otherwise unchanged. (CLAUDE.md has been updated with the corrected access model.)

[jonfroehlich]

Alternative for the Phase B read/analysis half: a superuser-gated admin "dedup" page (no IT dependency)

Building on the access-model correction above — since we can't reach the DB directly, the read/analysis half does not have to wait on a CSE-IT snapshot. We can expose the B1 analysis as a read-only admin view instead:

• Generates the per-person dedup table on the fly and offers a "Download CSV" button. Because the CSV streams through the authenticated request, it is sent only to the logged-in superuser's browser — it never lands on a web-served path (so it avoids the stale-dumped_data.json mistake) and needs no file egress.
• Runs inside the container (so it has DB access), behind the existing /admin login, gated on is_superuser (it exposes emails). Strictly read-only.
• Can also render a clustered HTML view (rows grouped by accent-folded normalized (first, last)) for quick eyeballing.
• Hook it into the existing MakeabilityLabAdminSite via get_urls().

This complements rather than replaces the rest of the plan:

• The destructive merge still ships as the reviewed, --dry-run-default, idempotent merge_duplicate_people entrypoint one-shot.
• A one-time read-only DB snapshot from CSE IT is still nice-to-have purely to rehearse that merge against real data locally — but it is now an optional safety margin, not a blocker. If IT replies promptly we use the snapshot; if not, the admin page unblocks analysis immediately.

Generalizable note: the no-shell-access constraint means we have no ad-hoc query capability against prod, so small read-only "data health" admin pages like this are a good general pattern here — this dedup page is just the first instance.

[jonfroehlich]

Resolved and verified on production in v2.11.0.

Outcome (prod Data Health, post-deploy)

• url_name collisions → 0 (was 10) — the We should figure out how to support two people with the same name as members of Makeability Lab #1206 /member/ 500 source is gone.
• duplicate-people → 2 (was 26) — the only rows left are the genuine namesakes Jasmine Zhang (id 840 → jasminezhang) and Jasmine Zhang (id 869 → jasminezhang2), kept separate with distinct, reachable URLs. 13 true-duplicate records were merged.

What shipped (PR #1327)

• merge_duplicate_people — decisions-file-driven, dry-run by default, applied once via a PROD-gated entrypoint one-shot. Relocates every relation via a generic Person._meta.get_fields() walk: publication/talk/poster/grant authorship and award recipients (sorted-M2M order preserved), positions, project roles, news, and the advisor/co-advisor/grad-mentor self-references. Backfills only-blank scalar fields, leaves the canonical record's image, atomic + idempotent, with a name-mismatch guard so a prod-keyed decisions file can't touch the wrong database.
• recompute_url_names + a Person.save() refactor — unique url_names with a readable middle-initial differentiator for namesakes (jasminexzhang) and numeric fallback; runs on every container start as the durable We should figure out how to support two people with the same name as members of Makeability Lab #1206 fix.
• member view hardened to a clean 404 instead of a 500 if a collision ever recurs.
• Bonus fix: normalize_person_name now folds accents via Unicode NFKD instead of a hand-maintained map that silently dropped acute accents/cedilla — this had mangled url_names (Cláudio Silva → cludiosilva) and hidden accented-name duplicates from this very dashboard. Fixed 5 url_names and surfaced one otherwise-invisible duplicate.

Reviewed merge decisions were validated end-to-end against a copy of the prod dump before shipping (589→576 people, collisions 10→0). Follow-up PR #1328 removed the spent one-shot from the entrypoint.

Closing as complete.