feat(singers): device-session provenance + smarter Merge modal (v0.91.0) - #204
Merged
Conversation
Motivated by the "two Chailas" incident: the KJ Singers list showed the same
person twice — one was her own self-registered singer ("Chaila R", from her
phone) and the other a duet-partner label ("Chaila") typed into someone else's
submission. Nothing distinguished a real device session from a duet label or a
KJ-hand-added entry, and Merge didn't say which singer was kept.
Backend:
- Capture the submitting device's User-Agent on /sing/submit and /change via a
new additive sing_requests.user_agent column.
- New ua_parse.py: best-effort UA -> friendly browser/OS/device (Android model).
- SingStore.get_requests_for_entries(): night-scoped linked-request lookup.
- routes._add_singer_session_info(): attaches a `session` block to each singer
(origin singer_ui / duet_partner / kj_added; has_device; device/phone/count),
wired into GET /rotation and every singer action response.
Frontend:
- 📱 icon only on real device-session singers; click opens a device-details
popup (parsed device, phone, first submitted, count, sources, raw UA).
- Merge modal replaces the inline dropdown: searchable singer list (device-linked
first), then a confirm step spelling out KEEP vs REMOVE, combined totals, a note
that a phone-linked keeper inherits history, and a real-device warning + Swap
that defaults the keeper to the device-linked singer.
Tests: ua_parse unit tests; session classification reproducing the incident;
UA persisted through /submit; e2e drive of the Merge modal. Docs + version bump.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Merge confirm dialog summed the two singers' sung/queued counts, which over-counts an entry both were credited on (e.g. a shared duet that collapses to one row after merge dedup). Compute from the deduped union of entries so the displayed post-merge totals match what the backend actually produces. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ua_parse: recognise EdgA/ (Edge on Android) before Chrome/; add a unit test. - merge e2e: drop the stale Swap comment, actually exercise Swap, and assert the real merge outcome (removed singer's row detaches) instead of the modal merely hiding (which happens regardless of merge success). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A bare Safari/ compatibility token appears in embedded WebKit clients (in-app WebViews) that aren't Safari. Require the documented Version/… Safari/ sequence so those aren't mislabelled. Add a test for a WebView UA. Co-Authored-By: Claude Opus 4.8 <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.
Why
Investigating the live "two Chailas" incident: the KJ Singers list showed the same person twice. One entry was her own self-registered singer ("Chaila R", submitted from her phone); the other was a duet-partner label ("Chaila") that another singer typed into their own submission. Nothing in the UI distinguished a real device session from a duet label or a KJ-hand-added entry, and Merge was a bare name dropdown that didn't say which singer was kept or what history moved.
What
Backend
User-Agenton/sing/submitand/sing/requests/<id>/change(new additivesing_requests.user_agentcolumn; auto-migrates on boot, safe to roll back).ua_parse.py: best-effort UA → friendly browser/OS/device (Android UAs expose the model, e.g.SM-S911B; iOS only "iPhone").SingStore.get_requests_for_entries(): night-scoped linked-request lookup (fails closed, mirrors the SMS phone lookup).routes._add_singer_session_info(): attaches asessionblock to each singer classifying origin assinger_ui(own device →has_device),duet_partner, orkj_added. Wired intoGET /rotationand every singer-action response.Frontend
Testing
ua_parse(incl. Edge-for-Android, embedded-WebView, iOS/Android/desktop);_add_singer_session_infoclassification reproducing the exact Chaila scenario; UA persisted through/submit.Deploy notes
systemctl restart kj-controller(interrupts playback — deploy between songs).Local CodeRabbit review completed and all findings addressed.
@coderabbitai ignore
🤖 Generated with Claude Code