fix(singers): merging singers unifies their identity so self-rename doesn't re-split (v0.98.0) - #209
Merged
Conversation
…(v0.98.0)
A singer who submitted from two browser sessions (two device_ids) under two
typed variants ("Jasmine" / "Jasmine!") was merged by the KJ into one displayed
singer — but the merge unified only the rotation ENTRIES, not the IDENTITY. When
she then renamed herself on her phone, /sing/rename rewrote only the songs that
one device owned (per-request edit_token), so the other session's song stayed
under the old name: she re-split, only one renamed.
A KJ merge is a deliberate assertion that these are one person, so a later
rename must carry the whole group:
- Merge now marks a canonical identity: /rotation/singer/merge calls
SingStore.mark_identity(target) in addition to persist_rename(source, target),
so the KEEP-side devices are recognised as the same identity too.
- Self-rename escalates for an established identity: /sing/rename renames the
WHOLE rotation name-group (rename_singer), migrates every aliased device
(remap_aliases) and rewrites tonight's requests (persist_rename) when
is_canonical_identity(old) — otherwise stays scoped to edit_token-owned
entries so two coincidental same-name walk-ins never rename each other.
New SingStore helpers: is_canonical_identity, remap_aliases, mark_identity.
Tests: 6 new unit + 3 new integration (TestMergedIdentitySelfRename).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ht scope
Two review findings on the identity-aware self-rename:
- Only KJ-established identities unlock a whole-group rename. singer_aliases
gains an `origin` column ('kj' vs 'self'); is_canonical_identity now requires
a 'kj' alias. A singer self-renaming their own song records a 'self' alias
that can NOT sweep up a coincidental same-name walk-in on a later rename.
'kj' is sticky (a self-rename never downgrades a managed identity). Pre-upgrade
rows default to 'self' (safe — behave exactly as before). Regression test:
double self-rename through a shared name leaves the other singer untouched.
- Fail closed when the night marker is unavailable. Self-rename only escalates
to persist_rename (which scopes the request rewrite by night) when a night
marker exists; otherwise it falls back to the edit_token-owned scope rather
than risk rewriting prior nights' history.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… fail-closed night in store - set_alias no longer keeps 'kj' sticky: a self-rename stamps 'self', so KJ authority never travels onto a name the singer later chose (can't launder a past merge into whole-group power over a coincidental walk-in). A genuinely KJ-merged multi-device identity stays canonical via its sibling devices. - mark_identity + persist_rename resolve the night marker internally and return 0 when none is available, rather than aliasing/rewriting every historical row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lias Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A singer submitted karaoke songs from two browser sessions (two stable
device_ids) under two typed name variants — "Jasmine" and "Jasmine!". The KJ merged them into one displayed singer on NomadPC. The merge correctly unified the rotation entries, but not the identity: when she then renamed herself on her phone,/sing/renameonly rewrote the songs that one device owned (per-requestedit_token), so the other session's song stayed under the old name — she re-split into two singers, only one renamed.A KJ merge is a deliberate assertion that these are one person, so a later rename must carry the whole group.
Fix
Aliases now carry provenance.
singer_aliasesgains anorigincolumn —'kj'(a KJ rename/merge established the identity) vs'self'(a singer's own/sing/rename). Only'kj'marks a canonical identity. A self-rename always stamps'self', so a singer can never launder a past merge into whole-group power over a coincidental same-name walk-in.Merge marks a canonical identity.
POST /rotation/singer/mergealready aliased the source devices onto the target (persist_rename); it now also callsSingStore.mark_identity(target), tagging the keep-side devices'kj'.Self-rename escalates for an established identity.
/sing/renamedecides per old-name: ifis_canonical_identity(old)and a night marker is available to scope the request rewrite, it 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 — two coincidental same-name walk-ins can never rename each other, and a missing night marker never clobbers prior nights.Migration safety
Additive
singer_aliases.origincolumn, defaults'self'on boot. Pre-upgrade rows can't be told apart, so they default to'self'(the safe choice — they behave exactly as before). A merge done after the upgrade writes fresh'kj'aliases, so the fix applies going forward.Tests
test_sing_store.py): the three helpers +'kj'-only identity + self-reset-of-authority, same-name/blank no-ops, device-less skips.test_sing_rename.py::TestMergedIdentitySelfRename): merge→self-rename renames the whole group; migrates all device aliases; the no-merge control stays scoped; a double-self-rename-through-a-shared-name that must not hijack the other singer.Deploy
Backend (
sing.py,sing_store.py,routes.py) → requiressystemctl restart kj-controller(deploy between songs). NomadPC may be mid-show — auto-deploy is paused during live shows, so this lands when the show ends.Reviewed locally with CodeRabbit (3 cycles, 2 majors + 1 minor addressed).
@coderabbitai ignore