fix(singers): merging singers unifies their identity so self-rename doesn't re-split (v0.98.0)

kj-controller/docs/CHANGELOG.md

@@ -4,6 +4,20 @@ Dated entries, newest first. Each entry notes any required deploy steps.
 
 ---
 
+## 2026-08-28 - Merging singers unifies their identity so self-rename doesn't re-split (v0.98.0)
+
+**Deploy:** backend (`sing.py`, `sing_store.py`, `routes.py`) → **requires `systemctl restart kj-controller`** (backend change; deploy between songs). Additive SQLite migration runs on boot (new `singer_aliases.origin` column, defaults `'self'`) — no manual step.
+
+- **Why:** a singer submitted from two browser sessions (two stable `device_id`s) under two typed variants — "Jasmine" and "Jasmine!". The KJ merged them into one displayed singer, which correctly unified the *rotation entries* but not the *identity*: when she then renamed herself on her phone, `/sing/rename` only rewrote the songs THAT one device owned (per-request `edit_token`), so the other session's song stayed under the old name and she **re-split** into two singers, only one renamed. A KJ merge is a deliberate assertion that these are one person — a later rename has to carry the whole group.
+- **What changed — aliases now carry provenance:** `singer_aliases` gains an `origin` column — `'kj'` when a KJ rename/merge established the identity vs `'self'` for a singer's own `/sing/rename`. `'kj'` is *sticky* (a later self-rename never downgrades it). Only `'kj'` aliases mark a **canonical identity**, so a singer self-renaming their own song can never gain the power to rename a coincidental same-name walk-in.
+- **What changed — merge marks a canonical identity:** `POST /rotation/singer/merge` already aliased the *source* devices onto the target (`persist_rename`); it now also calls `SingStore.mark_identity(target)`, tagging the target's own devices `'kj'` so the *keep* side is recognised as the same established identity too.
+- **What changed — self-rename escalates for an established identity:** `/sing/rename` now decides per old-name. If `SingStore.is_canonical_identity(old)` (a KJ `'kj'` alias exists) **and** a night marker is available to scope the request rewrite, the rename carries the **whole rotation name-group** (`rename_singer`), migrates **every** device aliased to the old name (`remap_aliases`), and rewrites tonight's requests (`persist_rename`). Otherwise it *fails closed* to the edit_token-owned scope exactly as before — so two coincidental same-name walk-ins can never rename each other, and a missing night marker never clobbers prior nights' history.
+- **New `SingStore` helpers:** `is_canonical_identity(name)` (a `'kj'` alias → this name?), `remap_aliases(old, new)` (re-point a whole identity group's device aliases), `mark_identity(name, night_started)` (tag tonight's devices under a name `'kj'`). `set_alias` gains an `origin` arg. All case-insensitive, best-effort, no-ops on blank input.
+- **Migration safety:** pre-upgrade alias rows can't be told apart, so they default to `'self'` — the safe choice (they behave exactly as before and never sweep up a same-name singer). A merge done after the upgrade writes fresh `'kj'` aliases, so the fix applies going forward.
+- **Tests:** 8 new unit tests (`test_sing_store.py` — the three helpers + `'kj'`-only identity + sticky-origin, incl. same-name/blank no-ops and device-less skips) + 4 new integration tests (`test_sing_rename.py::TestMergedIdentitySelfRename` — merge→self-rename renames the whole group, migrates all device aliases, the no-merge control that stays scoped, and a double-self-rename-through-a-shared-name that must not hijack the other singer). Full unit + integration suite green.
+
+---
+
 ## 2026-08-27 - Persistent singer rename — self-service + KJ-side both stick (v0.97.0)
 
 **Deploy:** backend (`sing.py`, `sing_store.py`, `rotation.py`, `rotation_store.py`, `routes.py`) + frontend (`static-sing/sing.js`, `sing.css`) → **requires `systemctl restart kj-controller`** (backend change; deploy between songs). Additive SQLite migration runs on boot (new `sing_requests.device_id` column + `singer_aliases` table) — no manual step.

kj-controller/pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "kj-controller"
-version = "0.97.0"
+version = "0.98.0"
 description = "Web-based karaoke show management with mpv + VLC playback"
 requires-python = ">=3.11"
 

kj-controller/routes.py

@@ -4362,7 +4362,17 @@ def merge_singers_route():
         return jsonify({"error": "source_name and target_name are required"}), 400
     try:
         rotation.merge_singers(source, target)
+        # Alias the SOURCE devices onto the target (persist_rename) AND record the
+        # TARGET's own devices as the same established identity (mark_identity), so
+        # a later self-service rename from EITHER side carries the whole merged
+        # group instead of re-splitting the singer.
         _persist_singer_rename(source, target)
+        store = getattr(current_app, 'sing_store', None)
+        if store is not None:
+            try:
+                store.mark_identity(target, night_started=store.get_night_started_at())
+            except Exception:
+                current_app.logger.exception("merge mark_identity failed")
         return _singer_action_response(rotation)
     except Exception as e:
         return jsonify({"error": str(e)}), 500

kj-controller/sing.py

@@ -1174,6 +1174,7 @@ def rename_me():
     # requests count — a device can never rename someone else's entries.
     entry_ids_by_old = {}
     verified_request_ids = []
+    verified_old_names = set()
     for it in items:
         if not isinstance(it, dict):
             continue
@@ -1190,21 +1191,45 @@ def rename_me():
             continue
         verified_request_ids.append(rid)
         old = (req.get("singer_name") or "").strip()
-        if (
-            old
-            and old.lower() != new_name.lower()
-            and req.get("status") == "approved"
-            and req.get("linked_entry_id")
-        ):
-            entry_ids_by_old.setdefault(old, []).append(req["linked_entry_id"])
-
-    # Rewrite the rotation entries the singer owns.
-    if rotation is not None:
-        for old, eids in entry_ids_by_old.items():
-            try:
-                rotation.rename_singer_in_entries(old, new_name, eids)
-            except Exception:
-                current_app.logger.exception("self-rename: entry rewrite failed")
+        if old and old.lower() != new_name.lower():
+            verified_old_names.add(old)
+            if req.get("status") == "approved" and req.get("linked_entry_id"):
+                entry_ids_by_old.setdefault(old, []).append(req["linked_entry_id"])
+
+    # Rewrite the rotation entries. Two modes, decided per old-name:
+    #
+    #  • Established identity (a KJ merged/renamed this singer into ``old``):
+    #    the singer is deliberately asserted to be ONE person, so a rename must
+    #    carry the WHOLE name-group across the rotation — not just the songs this
+    #    one device owns — else she re-splits under the stale name (the reported
+    #    "Jasmine" / "Jasmine!" bug). We also migrate every device aliased to
+    #    ``old`` and rewrite tonight's requests so no session reverts later.
+    #  • Otherwise (a plain typed name, no merge): stay scoped to edit_token-owned
+    #    entries so two coincidental same-name walk-ins never rename each other.
+    night_started = None
+    try:
+        night_started = store.get_night_started_at()
+    except Exception:
+        current_app.logger.exception("self-rename: night lookup failed")
+
+    for old in verified_old_names:
+        try:
+            # Escalate to a whole-group rename ONLY for a KJ-established identity
+            # AND only when we have a night marker to scope the request rewrite —
+            # without one, persist_rename would touch every historical request
+            # under this name, so we fail closed to the safe edit_token-scoped
+            # path rather than risk clobbering prior nights.
+            if store.is_canonical_identity(old) and night_started:
+                if rotation is not None:
+                    rotation.rename_singer(old, new_name)
+                store.persist_rename(old, new_name, night_started=night_started)
+                store.remap_aliases(old, new_name)
+            elif rotation is not None and old in entry_ids_by_old:
+                rotation.rename_singer_in_entries(
+                    old, new_name, entry_ids_by_old[old]
+                )
+        except Exception:
+            current_app.logger.exception("self-rename: entry rewrite failed")
 
     # Rewrite the verified requests' stored name (keeps provenance + the done
     # screen consistent, and means a pending request is approved under the new

kj-controller/sing_store.py

@@ -161,9 +161,16 @@ def init_schema(self):
             -- once and stores in localStorage. Persists across nights on purpose
             -- (a regular keeps their chosen name) — device_id is stable per
             -- browser, so there's no cross-night id-reuse hazard here.
+            -- `origin` (2026-08-28) records who established the alias: 'kj' for a
+            -- KJ rename/merge (a deliberate "these are one person" assertion) vs
+            -- 'self' for a singer's own /sing/rename. Only 'kj' aliases mark a
+            -- CANONICAL identity that a later self-rename may propagate across the
+            -- whole name-group — a singer self-renaming their own songs must never
+            -- gain the power to rename a coincidental same-name walk-in.
             CREATE TABLE IF NOT EXISTS singer_aliases (
                 device_id      TEXT PRIMARY KEY,
                 canonical_name TEXT NOT NULL,
+                origin         TEXT NOT NULL DEFAULT 'self',
                 updated_at     TEXT NOT NULL DEFAULT (datetime('now', 'localtime'))
             );
             """
@@ -221,6 +228,20 @@ def init_schema(self):
         except sqlite3.OperationalError as e:
             if "duplicate column name" not in str(e).lower():
                 raise
+        # Additive migration — `singer_aliases.origin` (2026-08-28). Distinguishes
+        # KJ-established identities ('kj', from a rename/merge) from a singer's own
+        # self-rename ('self'). Only 'kj' unlocks a whole-group rename. Pre-upgrade
+        # rows can't be told apart, so they default to 'self' — the SAFE choice:
+        # they behave exactly as before (edit_token-scoped renames) and never gain
+        # the power to sweep up a coincidental same-name singer. A merge done after
+        # the upgrade writes fresh 'kj' aliases, so the fix applies going forward.
+        try:
+            conn.execute(
+                "ALTER TABLE singer_aliases ADD COLUMN origin TEXT NOT NULL DEFAULT 'self'"
+            )
+        except sqlite3.OperationalError as e:
+            if "duplicate column name" not in str(e).lower():
+                raise
         conn.commit()
 
     # ------------------------------------------------------------------
@@ -740,20 +761,33 @@ def get_alias(self, device_id):
         ).fetchone()
         return row[0] if row else None
 
-    def set_alias(self, device_id, canonical_name):
-        """Upsert a device → canonical-name mapping. No-op on blank input."""
+    def set_alias(self, device_id, canonical_name, origin="self"):
+        """Upsert a device → canonical-name mapping. No-op on blank input.
+
+        ``origin`` is 'kj' when a KJ rename/merge established the identity, else
+        'self' (a singer's own /sing/rename). The stored origin always reflects
+        the LAST writer: a KJ action stamps 'kj'; a self-rename stamps 'self'.
+        Crucially, KJ authority does NOT travel with a device onto a name the
+        SINGER later chose — a self-rename that changes the name resets origin to
+        'self', so a singer can never launder a past merge into whole-group power
+        over a coincidental same-name walk-in. Only 'kj' aliases satisfy
+        is_canonical_identity; a genuinely KJ-merged multi-device identity stays
+        canonical via the sibling devices the KJ/merge path re-stamps 'kj'.
+        """
         device_id = (device_id or "").strip()
         canonical_name = (canonical_name or "").strip()
+        origin = "kj" if origin == "kj" else "self"
         if not device_id or not canonical_name:
             return
         conn = self._get_conn()
         conn.execute(
-            "INSERT INTO singer_aliases (device_id, canonical_name, updated_at) "
-            "VALUES (?, ?, datetime('now', 'localtime')) "
+            "INSERT INTO singer_aliases (device_id, canonical_name, origin, updated_at) "
+            "VALUES (?, ?, ?, datetime('now', 'localtime')) "
             "ON CONFLICT(device_id) DO UPDATE SET "
             "  canonical_name = excluded.canonical_name, "
+            "  origin = excluded.origin, "
             "  updated_at = datetime('now', 'localtime')",
-            (device_id, canonical_name),
+            (device_id, canonical_name, origin),
         )
         conn.commit()
 
@@ -768,6 +802,86 @@ def clear_alias(self, device_id):
         )
         conn.commit()
 
+    def is_canonical_identity(self, name):
+        """True if ``name`` is a KJ-established singer identity.
+
+        An identity is "established" only when at least one device carries a
+        KJ-origin alias to ``name`` — i.e. a KJ renamed/merged someone into it.
+        This is the trust anchor that lets a self-service rename safely carry the
+        WHOLE name-group rather than only the calling device's own songs.
+
+        A singer's OWN /sing/rename records a 'self'-origin alias, which does NOT
+        count here — otherwise a singer who self-renamed their song to "Mike"
+        could, on a second rename, sweep up a coincidental second "Mike" walk-in
+        whose edit_token they never held. Only the KJ's explicit assertion that a
+        name is one managed identity unlocks whole-group renames.
+        """
+        name = (name or "").strip()
+        if not name:
+            return False
+        conn = self._get_conn()
+        row = conn.execute(
+            "SELECT 1 FROM singer_aliases "
+            "WHERE LOWER(canonical_name) = LOWER(?) AND origin = 'kj' LIMIT 1",
+            (name,),
+        ).fetchone()
+        return row is not None
+
+    def remap_aliases(self, old_name, new_name):
+        """Re-point every device aliased to ``old_name`` at ``new_name``.
+
+        Used when a merged identity is renamed: all the devices the KJ merged
+        into ``old_name`` must follow the singer to ``new_name`` so none of them
+        re-splits under the stale name on a future submission. Cross-night like
+        the aliases themselves (device_id is stable per browser). No-op on blank
+        input or a no-op rename.
+        """
+        old_name = (old_name or "").strip()
+        new_name = (new_name or "").strip()
+        if not old_name or not new_name or old_name.lower() == new_name.lower():
+            return
+        conn = self._get_conn()
+        conn.execute(
+            "UPDATE singer_aliases SET canonical_name = ?, "
+            "  updated_at = datetime('now', 'localtime') "
+            "WHERE LOWER(canonical_name) = LOWER(?)",
+            (new_name, old_name),
+        )
+        conn.commit()
+
+    def mark_identity(self, name, night_started=None):
+        """Alias every device that submitted under ``name`` tonight → ``name``.
+
+        Records the devices behind a name as one established identity (a
+        KJ-origin alias). Called on the KEEP side of a KJ merge so the merged
+        singer is a recognised identity even from a device that never had to be
+        renamed — which is what makes a later self-service rename carry the group.
+        Best-effort; returns the number of devices marked.
+
+        Night-scoped like the rest of request/phone resolution. The marker is
+        resolved internally when not passed; with no night at all we fail closed
+        (return 0) rather than mark every historical device under ``name``.
+        """
+        name = (name or "").strip()
+        if not name:
+            return 0
+        night_started = night_started or self.get_night_started_at()
+        if not night_started:
+            return 0
+        conn = self._get_conn()
+        params = [name, night_started]
+        night_clause = " AND created_at >= ?"
+        rows = conn.execute(
+            "SELECT DISTINCT device_id FROM sing_requests "
+            "WHERE LOWER(singer_name) = LOWER(?)"
+            "  AND device_id IS NOT NULL AND device_id != ''" + night_clause,
+            tuple(params),
+        ).fetchall()
+        device_ids = [r[0] for r in rows]
+        for did in device_ids:
+            self.set_alias(did, name, origin="kj")
+        return len(device_ids)
+
     def persist_rename(self, old_name, new_name, night_started=None):
         """Make a KJ/merge rename of ``old_name`` → ``new_name`` sticky.
 
@@ -779,20 +893,22 @@ def persist_rename(self, old_name, new_name, night_started=None):
            the current rotation-entry name) keeps working after the rename.
 
         Night-scoped (``created_at >= night_started``) like the rest of the
-        request/phone resolution to avoid touching prior nights' history. Returns
-        the number of distinct devices aliased. Best-effort; safe to call for a
-        name that has no portal submissions (returns 0).
+        request/phone resolution to avoid touching prior nights' history. The
+        marker is resolved internally when not passed; with no night at all we
+        fail closed (return 0) rather than rewrite every historical request under
+        ``old_name``. Returns the number of distinct devices aliased. Best-effort;
+        safe to call for a name that has no portal submissions (returns 0).
         """
         old_name = (old_name or "").strip()
         new_name = (new_name or "").strip()
         if not old_name or not new_name or old_name.lower() == new_name.lower():
             return 0
+        night_started = night_started or self.get_night_started_at()
+        if not night_started:
+            return 0
         conn = self._get_conn()
-        params = [old_name]
-        night_clause = ""
-        if night_started:
-            night_clause = " AND created_at >= ?"
-            params.append(night_started)
+        params = [old_name, night_started]
+        night_clause = " AND created_at >= ?"
         rows = conn.execute(
             "SELECT DISTINCT device_id FROM sing_requests "
             "WHERE LOWER(singer_name) = LOWER(?)"
@@ -801,7 +917,8 @@ def persist_rename(self, old_name, new_name, night_started=None):
         ).fetchall()
         device_ids = [r[0] for r in rows]
         for did in device_ids:
-            self.set_alias(did, new_name)
+            # A KJ/merge rename establishes a managed identity (origin='kj').
+            self.set_alias(did, new_name, origin="kj")
         # Rewrite the tonight requests' primary singer_name for provenance.
         conn.execute(
             "UPDATE sing_requests SET singer_name = ? "