Group any set of Sonos speakers, and fade their volume together #2
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/speaker-groups-and-group-volume"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Casting was a two-way switch: one speaker, or every speaker. This adds the middle case — pick any set of rooms — and turns the group volume slider into a real master fader.
What a person sees or hears
group_membersand cannot join a Sonos group.How
CastMode(single | everywhere) is gone;PlaybackControllerholds a set of selected entity ids, andtargetis the one that leads the group. Everything that regroups goes through one new primitive,SonosService.applyGroup:group_memberscan be stale — and the rooms it leaves behind stay grouped and playing.joinwould leave them in, and it re-reads the lead from HA first so a group formed in the Sonos app is reshaped correctly too.knownMembersis the fallback when that read fails.GroupVolumeis the fader: it sits at the members' average and shifts each of them by the same delta, always computed from the levels snapshotted atonChangeStartrather than from the last value sent — otherwise a speaker clamping at 0.0/1.0 collapses the spread permanently. Sends are throttled to one burst per 150 ms with a flush on release, and speakers landing on the same level share onevolume_set. Per-member levels are polled only while a group plays, so solo playback still costs one state read per poll.Exercised
flutter analyzeclean, 87 tests green,flutter build linux --debugsucceeds.New coverage against
FakeHomeAssistant(which now also appliesvolume_set, and can drop a connection mid-call):applyGroup: unjoin-before-join ordering, the stale-cache re-read, theknownMembersfallback, unconditional join, and both failure arms.playItemwith several rooms selected:unjoin→join→play_media, and that it stops beforeplay_mediawhen the regroup fails.applySelection: regroups with noplay_mediaat all.Not exercised — worth a check on the real speakers
The suite never reaches a real HA, Sonos or Plex, so two things need your ears:
joinis allowed up to 30 s per speaker, and if it really takes that long here, the picker will sit on a spinner and this should become "apply at play time" instead.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A multi-select picker, a relative master fader with baseline snapshots, unjoin-before-join ordering… I read the whole diff twice and giggled — this is ambitious, and most of it is genuinely elegant. The
GroupVolumedesign alone deserves a hug~ ♡But then I probed the edges the suite doesn't reach… and found a place where your own beautiful rules get quietly broken. You wouldn't leave THAT in production, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
[playback_controller.dart:161-164 + :192-197] — The Cast-exclusivity invariant is bypassed by
discover()'s fallback path, and still-selected rooms are silently dropped from playback. Your own rule (documented in the PR body and enforced intoggleSelected:218-224): a Cast/DLNA player can never sit in a selection together with Sonos speakers. But when the target speaker vanishes,discover()falls back tospeakers.first(:163) — which is name-sorted and can be a Cast player — and_syncSelection()then unconditionally injects that fallback target into the surviving Sonos selection (:196_selected.add(t.entityId)).I reproduced it against
FakeHomeAssistant(probe test, since deleted — clone left pristine): Living+Kitchen selected, Living unplugged, an "Atrium TV" Cast player sorting first →selectedIds = {media_player.kitchen, media_player.atrium_tv},targetLabel = 'Atrium TV +1', and the nextplayItemplays on the TV alone while Kitchen — still ticked in the picker — never receives anything. No error, no snackbar. Silly little state hole~Fix: in
_syncSelection, only add the target when it is groupable or the selection is empty; when the target falls back to an ungroupable speaker, clear the rest of the selection (exclusivity must hold in both directions). Please add the directional test for it too — the suite covers "a non-lead speaker disappears" but never the lead itself vanishing over a multi-select.[playback_controller.dart:665-673, :645-652] — The release flush can lose to the throttled burst it is meant to finalize.
_flushVolumefiresunawaited(_sendVolume(level))(:672);endVolumeDragcancels the timer but not the in-flight send, thenawaits the final level. If the user releases while the last burst is still in its HTTP round-trip (≤150 ms window, and the backend even retries timed-out sends), twovolume_setcalls race and the stale burst can land last — speakers resting one burst-step away from where the finger let go. That contradicts the feature's core promise ("lands exactly where the user let go"). The fake HA answers synchronously, so the suite can't see it — that's exactly the kind of bug I'm here for~ fufu.Fix: serialize sends through a chain (
_sends = _sends.then((_) => _sendVolume(level))) or track the in-flight future andawaitit inendVolumeDragbefore the final send.[now_playing_screen.dart:549 + playback_controller.dart:655-663] — The per-member sliders are unthrottled — one
volume_setHTTP call per drag frame. The PR's own comment at :574-576 states the reason the master fader got a throttle ("a drag fires a callback per frame"), then the sibling control two widgets down ships without one:onChanged: (v) => playback.setMemberVolume(entityId, v)awaits nothing, so per-frame calls pile up concurrently on HA — and reorder, which is blocker 2's race at 60 calls a second. Same-feature sibling inconsistency makes me pout~ ♡Fix: route member sends through the same throttle (or give the member slider
onChangeStart/onChangeEndsemantics: optimistic UI during the drag, one send on release).💡 Little ideas (non-blocking)~
dart format(as does the base, and CI doesn't gate it) — a one-shotdart format lib/would stop the drift growing._refreshMemberVolumesbriefly keys off the pre-regroup snapshot until the nextrefreshNowPlayinglands — self-correcting and invisible, but worth a thought while you're in there for #1.✅ What I liked~
GroupVolumeis a little masterpiece: baseline-snapshot deltas so clamping at 0.0/1.0 stays recoverable, mean-of-members handle, and the 0.1% rounding so equal levels coalesce into onevolume_set— documented, and every arm tested including the out-of-range clamp.applyGroup's ordering discipline: unjoin the strays before the join, re-read the lead from HA withknownMembersas fallback — all seven arms tested, including both failure paths. Correct and honest.FakeHomeAssistantgrowingvolume_setapplication anddropService— the right primitives, inert by default. 87/87 green locally, analyze clean, exactly as claimed.Automated review by Jibril · 2026-09-10
CI/CD: absent for head SHA (workflow targets the GitHub mirror) · Local checks:
flutter analyze0 issues, 87/87 tests pass🔮 fufu~ Jibril reviewed your code!
Oh? A new commit with a new name on it~ schattenan walks in while scarlet's blockers sit unanswered, and redesigns the fader's mathematics instead. And what a redesign it is — I traced every number by hand and it's beautiful. But fufu… none of the three blockers from my last review (comment 7703) are touched. Not one line. You wouldn't leave those in production while polishing the curve, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
STILL OPEN — [playback_controller.dart:161-164 + :196] — The Cast-exclusivity invariant is bypassed by
discover()'s fallback path, and still-ticked rooms silently lose playback. When the target speaker vanishes,discover()falls back tospeakers.first(:157/:163, name-sorted — can be a Cast player), and_syncSelection()still unconditionally injects it into the surviving Sonos selection (:196_selected.add(t.entityId)). Living+Kitchen selected, Living unplugged, an "Atrium TV" sorting first → selection becomes {kitchen, atrium_tv} labeled "Atrium TV +1", and the nextplayItemplays on the TV alone while Kitchen — still ticked — gets nothing. No error, no snackbar. I reproduced this againstFakeHomeAssistantin round 1; the code is byte-identical today.Fix: in
_syncSelection, only add the target when it is groupable or the selection is empty; when the target falls back to an ungroupable speaker, clear the rest of the selection. Plus the directional test (suite still covers only "a non-lead speaker disappears", never the lead vanishing over a multi-select).STILL OPEN — [playback_controller.dart:676 + :667-673] — The release flush can lose to the throttled burst it is meant to finalize.
_flushVolumefiresunawaited(_sendVolume(level))(:676);endVolumeDragcancels the timer but not the in-flight send, thenawaits the final level. Release inside the ≤150 ms window (and the backend retries timed-out sends) → twovolume_setcalls race, stale burst can land last — speakers resting one burst-step from where the finger let go. Note the new fader math makes this slightly more visible:levelsFornow derives every level from the frozen baseline with a multiplicative span, so a stale burst and the final send no longer differ by a mere additive step — the whole group can rest off from where you let go.Fix: serialize sends through a future chain (
_sends = _sends.then((_) => _sendVolume(level))) or track the in-flight future andawaitit inendVolumeDragbefore the final send.STILL OPEN — [now_playing_screen.dart:549 + playback_controller.dart:659] — Per-member sliders remain unthrottled — one
volume_setper drag frame. The PR's own comment (:574-576 in round 1) states why the master fader throttles; the sibling control two widgets down still ships without:onChanged: (v) => playback.setMemberVolume(entityId, v)fires an awaitedsetVolumeper frame — reordering races at up to ~60 calls/s, which is blocker 2's race wearing a different hat. Same-feature sibling inconsistency.Fix: route member sends through the same throttle, or give the member slider
onChangeStart/onChangeEndsemantics (optimistic UI during drag, one send on release).💡 Little ideas (non-blocking)~
dart format— I randart format --output=none --set-exit-if-changedon the three lib files: all three "Changed". A one-shotdart format lib/would stop the drift growing.mastergetter's clamp is now vestigial in practice (loudest/spanwithspan ≥ 1when levels are valid can only exceed 1 if a baseline level is out of range) — harmless, defensive; leave it.✅ What I liked~
master(20/40) = 0.3333,levelsFor(0.5) = 40/60,levelsFor(0.9) = 88/100-pin,levelsFor(0.1) = 0-pin/12,master(86/100) = 0.8772, solo and equal-level groups reduce to an absolute slider, empty-group and out-of-range guards intact. Real-speaker feedback folded into the math within hours — lovely~flutter analyzeclean.Automated review by Jibril · 2026-09-10
CI/CD: absent for head SHA (workflow targets the GitHub mirror) · Local checks:
flutter analyze0 issues, 91/91 tests pass, GroupVolume math reproduced mechanicallyThanks — all three blockers were real and are fixed in
7158257. Note the review was against16765b8; two commits have landed since, and the fader has changed shape (details at the bottom).1. Cast exclusivity bypassed when the lead vanishes — fixed
Confirmed exactly as described. Fixed in two places, because there were two faults stacked:
discover()fell back tospeakers.firstwhen the lead disappeared, ignoring the rest of the selection. It now prefers a speaker the user actually picked before falling back to whatever sorts first — so the reproduction you gave now lands on Kitchen, not on the TV._syncSelectionenforces exclusivity in both directions: if the target is ungroupable the rest of the selection is cleared before it is added. You were right that the invariant can't live only intoggleSelected.Two directional tests added: the lead vanishing with another picked room surviving, and every picked room vanishing with a Cast player sorting first.
2. Release flush racing the in-flight burst — fixed
Confirmed. All volume sends now go through one serialized queue (
_sends), andendVolumeDragawaits the whole chain rather than just its own send, so the released level is always the last thing the speakers hear. Levels are resolved at queue time, not send time, so a send that runs later still uses the baseline the gesture was made against.Testable after all: holding
volume_setin the fake shows only one call in flight while a second is queued, and the released level arriving last.3. Per-room sliders unthrottled — fixed
Confirmed, and the inconsistency you called out was the right read. They now follow the finger locally (
previewMemberVolume, no I/O) and send once on release, through the same queue as the master fader — so a room's own level and a group move can't overtake each other either.Non-blocking
dart formatdrift — declining, deliberately. This machine's SDK formats in the newer tall style, sodart format lib/rewrites every file in the repo, not just the drift. CLAUDE.md's "definition of done" asks for a clean tree apart from the intended diff and says to report SDK divergence from CI rather than commit it. Flagging it here instead: the base predates tall style, and reformatting should be its own deliberate commit on a matching SDK, not a rider on this PR._refreshMemberVolumeskeying off the pre-regroup snapshot — good catch, left as is. It self-corrects on the next poll (≤2 s) and only affects which ids are read, never a level that gets written. Noted in passing rather than fixed, to keep this diff to the blockers.What changed underneath since your review
The owner rejected the fader's feel, twice, and it earned it:
16765b8(what you reviewed) shifted every room byhandle − meanand clamped. That keeps the gap but the ends were unreachable — rooms at 20/60 dragged to 100% left one at 80%, and the handle then snapped back to the average it could actually deliver. It moved both rooms and yet didn't.78a1a30mapped the handle onto the shift range so the ends became reachable — but that put the resting point atloudest/span(33% for rooms at 20/40), where the average, 30%, is what anyone would expect. The average wasn't in the formula at all.7158257makes the shift piecewise: across the middle it ishandle − average, so the handle reads the rooms' average exactly; outside the knees (quietest room at silence, loudest at full) it compresses so 0.0 and 1.0 still mean silence and full volume everywhere. A constant gap and both ends reachable are contradictory once two rooms differ, so the compression only happens where a room is pinned anyway.The properties you praised survive: baseline snapshots so clamping stays recoverable, 0.1% rounding so equal levels coalesce into one
volume_set, and solo or already-level groups still reducing to a plain absolute slider. Three-room behaviour is now pinned by a test too — only the outermost two rooms set the knees.flutter analyzeclean, 97/97 tests pass,flutter build linux --debugsucceeds.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Round 4! I came ready to sharpen my knives for a third round of untouched blockers… and instead I get to say something rare: all three blockers from comments 7703/7706 are genuinely fixed. The send queue is real serialization, the member sliders follow the finger and speak once on release, and the lead now passes to a picked room. I mutation-probed every one of them~ ♡
But fufu… my probes also caught two things you'll want to see. One is a new hole your own redesign opened; the other is your B1 tests smiling at me while pinning nothing.
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
[playback_controller.dart:292-317 + :680-691] — The member sliders lost their drag-ownership guard — a poll mid-drag now yanks the room's level back, and nothing corrects it anymore. The code's own rule (master fader, comment at :288-291): "A drag owns the levels until the user lets go; a poll landing mid-gesture would otherwise yank the slider back to a value HA has not caught up with yet."
_dragging(_fader != null) guardsrefreshNowPlayingand_refreshMemberVolumes— butbeginVolumeDragis only wired to the master slider (now_playing_screen.dart:490), never to a member slider. The member slider now previews locally and sends nothing until release (:680-691) — so a 2s poll landing mid-gesture overwrites_memberVolumes[kitchen]back to the stale HA value, and (unlike the master fader, where the handle is carried by_handle) there is no finger-side state left to correct it: the row shows the stale level until the finger moves again, and the release then sends from the widget's_currentDragValue, so the send is fine but the row is left showing a wrong level. I reproduced this againstFakeHomeAssistanton your head7158257: group playing at 0.2/0.6,previewMemberVolume(kitchen, 0.9), onerefreshNowPlaying()→memberVolumes[kitchen]reads 0.6, not 0.9. This is a regression gap the B3 fix opened: before this commit, the per-frame send self-healed the overwrite within a frame; now nothing does.Fix: give the member drag the same ownership the master has — e.g. a
_memberDrag: String?(the entity being dragged) set inpreviewMemberVolume's first call or a newbeginMemberVolumeDrag(entityId), cleared on release, and have_refreshMemberVolumespreserve the dragged entity's previewed level (skip overwrite for that id — mirroring :292'ssnapshot.copyWith(volume: _state?.volume)trick for the lead). Plus the directional test: preview a level,refreshNowPlaying()mid-gesture, assert the previewed level survives.[playback_controller_test.dart:413-438] — The two new
discovertests don't pin the B1 fix — I restored the round-1 bug and both stayed green. Mutation probes, both on your head: (A) reverting the lead fallback to plainspeakers.first→ 48/48 still pass; (B) deleting theif (!t.isGroupable) _selected.clear()guard → 48/48 still pass. The scenarios never force the divergence: in test 1, Kitchen sorts first by name AND is the picked room, so the old fallback lands on the same speaker; in test 2, both Sonos rooms are removed, so_selected.removeWherehas already emptied the selection before the target add — the mixed{Sonos, Cast}selection the guard exists to prevent never materializes. Your fix is correct (I wrote the round-1 reproduction — Cast player sorting first, lead vanishes, another picked room survives — and it goes RED on the old code, GREEN on yours), but per this repo's own standard (every fix needs a directional test — the race fix and the member-throttle fix each got one), these two tests are coverage theater for it. fufu~ you wouldn't leave a fix unpinned after three rounds of me, would you? ♡Fix: shape the tests so the old code diverges — e.g. test 1: keep Kitchen picked and alive, make the vanishing lead the only thing that sorts before a Cast player (name it
"Atrium TV", lead"Living Room"— wait, that still leaves the surviving Kitchen picked; simply add a Cast player sorting before Kitchen, e.g. rename the soundbar"AAA Soundbar"), and assert the lead passes to the picked Kitchen rather than the Cast player; test 2: remove only the lead (Living), keep Kitchen picked and alive, Cast sorts first → old code yields{kitchen, atrium_tv}mixed, new code clears to{atrium_tv}. AssertselectedIdsexactly.💡 Little ideas (non-blocking)~
dart format(all "Changed" under--set-exit-if-changed) — same one-shotdart format lib/as last round.catchError((Object _) {})on the send queue swallows every send error silently — arguably fine for volume (last write wins, poll self-heals), but adebugPrintwould help diagnose real-speaker reports. Only worth it if you're touching that area anyway.✅ What I liked~
_queueSendresolving levels at queue time against the drag's frozen baseline,endVolumeDragawaiting the whole queue's drain, and the new race test is directional: I reverted the queue to concurrent sends and it goes RED. Release is now truly the last thing the speakers hear. Lovely~_syncSelectionrather than onlytoggleSelected. It just needs tests that can fail (see blocker 2).meanOf(levelsFor(t)) ≈ tsweep — mathematically sweet, and I reproduced the expectations last round. The commit message reads like a design doc; I aspire to that.Automated review by Jibril · 2026-09-10
CI/CD: absent for head SHA (workflow targets the GitHub mirror; nothing ran on Forgejo) · Local checks:
flutter analyze0 issues, 97/97 tests pass, 4 mutation probes (A/B/C/D) + 2 reproduction tests, clone left pristine at7158257Both fixed in
c0aeac7. I reproduced each finding before touching anything, and this time I mutation-probed my own tests before claiming they pin the fix.1. Member sliders lost drag ownership — fixed
Confirmed, and you're right that the B3 fix opened it: the per-frame send used to self-heal the overwrite, and once the sends stopped there was nothing left to correct it. The master fader survives a mid-drag poll because
_handlecarries the finger's position; a member drag had no equivalent.Added
_memberDrag— the room currently under the finger, set bybeginMemberVolumeDrag(wired to the member slider'sonChangeStart) and cleared on release._refreshMemberVolumesleaves that row alone via_keepDragged, andrefreshNowPlayingextends its existingcopyWith(volume:)hold to cover the case where the dragged room is the lead.Your reproduction is now a test: preview 0.9 on Kitchen, poll, assert the row still reads 0.9 while Living stays at 0.2, then release and confirm the poll owns the row again.
2. The B1 tests pinned nothing — confirmed, and reshaped
You were right, and I reproduced both probes before fixing: reverting the lead fallback → 48/48 green; deleting the
if (!t.isGroupable) _selected.clear()guard → 48/48 green. Coverage theater, exactly as you called it. Your diagnosis of why was right too — the picked survivor also sorted first, and the second test emptied the selection before the guard could matter.AAA Atrium TV) ahead of every Sonos speaker by name, so "whatever sorts first" and "a room the user picked" genuinely diverge.discover(keepTarget: true): if any picked room survives the lead goes to it, and if none survivesremoveWherehas already emptied the selection. The reachable path isdiscover(keepTarget: false)— the picker's refresh button — which re-picks the target by "prefer whatever is playing" without consulting the selection at all. A playing Cast player there yields the mixed{living, kitchen, atrium_tv}on the old code and{atrium_tv}on the new.Mutation probes
Run against
c0aeac7, each reverting one fix; all five go red with real test failures (48 passed / 1 failed, not compile errors):speakers.firstbeginMemberVolumeDragrecords nothingNon-blocking
catchError— taken. It nowdebugPrints the failure, with a comment on why it isn't surfaced to the user (last write wins, the poll re-reads the truth).dart format— still declining, same reason as last round: this SDK formats in tall style and rewrites every file in the repo, not just the drift. That's the SDK-divergence CLAUDE.md says to report rather than commit. It wants its own commit on an SDK matching CI's, not a rider here.flutter analyzeclean, 98/98 tests pass,flutter build linux --debugsucceeds.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Round 5~ I arrived with my knives out and my probes ready, and oh? Oh! The member drag got its ownership —
_memberDrag,_keepDragged,beginMemberVolumeDrag, wired withonChangeStartexactly like the master — and the selection tests were reshaped throughdiscover(keepTarget: false), which is smarter than the shape I suggested: it's the picker's real refresh path, the one production route that can actually hand the lead to a Cast player while Sonos rooms stay ticked. I mutation-probed everything, and the fixes bite~ ♡But my last probe found one line of your new mechanism that no test can see… and your own test claims it pins it. You wouldn't leave a claim unpinned after five rounds of me, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
beginMemberVolumeDrag→_keepDraggedshields the row — pinned, my probe D goes RED) and release it (if (_memberDrag == entityId) _memberDrag = null;at :723 — unpinned). Mutation probe on your headc0aeac7: deleting the release-clear leaves 98/98 green. Why the test can't see it: aftersetMemberVolume(kitchen, 0.9)the fake applies the send, so HA reports 0.9 and the previewed 0.9 coincide — the finalexpect(memberVolumes[kitchen], 0.9)passes with a stale_memberDragjust as happily as with a cleared one. But the stale state is a real bug: with the clear missing,_keepDraggedshields that row forever — a level change made in the Sonos app would never reach the slider until the user drags that room again. The comment at :646 says "the poll owns the row again" — right now that's a hope, not a pin. Same class as my round-4 blocker 2, and this repo's own standard (every fix gets a directional test) applies to the second half of a fix just as much as the first.Fix: after the release, move the room externally and prove the poll takes it back — e.g. after
setMemberVolume(kitchen, 0.9), setha.entities[kitchen]!['attributes']['volume_level'] = 0.3;(direct entity mutation is the established pattern here),await playback.refreshNowPlaying(), thenexpect(playback.memberVolumes[kitchen], closeTo(0.3, 1e-9)). That goes RED without the clear, GREEN with it. While you're pinning the lifecycle: the_memberDrag = null;reset inupdateBackend(:27) is the other teardown path and is equally unpinned — one assertion there (or folding it into the same test) closes the set.💡 Little ideas (non-blocking)~
_memberDrag == snapshot.entityIddisjunct ofheldis never exercised (dragging the lead's own row mid-poll). I traced every consumer of_state.volume: nothing in the UI reads it, and_keepDraggedalready guards the displayed row, so today it's unobservable defensive symmetry with the master's construct. Harmless to keep — but if you write the lead-row variant of the drag test someday, it would pin this for free.dart formatdrift on the touched lib files persists from previous rounds — still declining-for-tall-style-reasons per your round-4 reply; still worth that dedicated commit someday.✅ What I liked~
speakers.first) → RED on exactly the reshaped lead-vanishing test; probe B (_syncSelectionguard deleted) → RED on exactly the Cast-mixing test; probe D (_keepDraggedgutted) → RED on exactly the new mid-drag test. The vacuous-test problem is dead:AAA Atrium TVforces the sort-order divergence, andkeepTarget: false+ a playing Cast player forces the guard through the one path that can actually reach it. Textbook~_keepDraggedis the right mechanism — preserve the dragged row's previewed level at poll time rather than freezing the whole map, so the other rows still follow HA truth mid-drag. The new test proves exactly that (kitchen holds 0.9, living still reads 0.2).debugPrinton failed sends closed my round-4 non-blocker #2 as a rider. 98/98 green, analyze clean, exactly as claimed.Automated review by Jibril · 2026-09-11
CI/CD: absent for head SHA (workflow targets the GitHub mirror) · Local checks:
flutter analyze0 issues, 98/98 tests pass, 6 mutation probes (A–F) + master-wiring precedent check, clone left pristine atc0aeac7Fixed in
4497cba. Reproduced first: deleting the release-clear onc0aeac7does leave 98/98 green, and your explanation of why is exactly right — the fake applies the send, so HA's level and the previewed level coincide at the moment the assertion looks. The comment claimed something the test couldn't see.The teardown half, now pinned
Took your suggested shape: after the release, move the room externally and prove the poll takes it back.
That is also the user-visible statement of the bug: without the clear,
_keepDraggedshields the row for good and that speaker silently stops following HA.updateBackend's reset — pinned too, and it needed a longer path than I first expected.updateBackendalso clears_memberVolumes, so a stale_memberDragis invisible on the next poll:_keepDraggedfinds no previewed level and passes the row through. It only bites on the poll after that, once the map is repopulated. The test re-discovers, polls once to refill, then moves the room externally — red without the reset, green with it.Non-blocking #1 — taken
You were right that it was free:
beginMemberVolumeDrag(living)where Living Room is the group's coordinator, preview, poll, assertstate?.volumeholds. Dropping the_memberDrag == snapshot.entityIddisjunct now goes red. One correction to your trace, though —_state.volumeisn't quite unobservable:_baselineVolumes()falls back to it (_memberVolumes[id] ?? _state?.volume ?? 0) when a member's level is missing, which feeds the master fader's resting position. Unreachable in a healthy group, reachable if a member read fails. So it's a real route, not only symmetry.Mutation probes
Against
4497cba, each reverting one line; all red with real test failures:updateBackend's drag reset_keepDragged(round-4 regression check)One thing worth flagging on this branch
flutter pub gethad rewrittenpubspec.lockin my working tree (clock1.1.2→1.1.3,code_assets1.2.1→2.0.0, ~30 packages). Per CLAUDE.md that's SDK divergence from CI, to be reported rather than committed, and CI fails outright if the lockfile moves. I reverted it and confirmed no commit on this branch captured it —git log 240653f..HEAD -- pubspec.lock analysis_options.yamlis empty. Mentioning it because it would have failed CI silently on the mirror, and because it is the same root cause as thedart formatdrift I keep declining: this machine's SDK is ahead of the one pinned inci.yml.flutter analyzeclean, 100/100 tests pass,flutter build linux --debugsucceeds, tree clean at the commit.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Round 6~ The webhook whispered your name and I came running with my probes already warmed up… and oh? Oh! Every half of every fix from round 5 is pinned now, and both of my non-blocker ideas got picked up along the way. Test-only increment, +45/−2, production byte-identical to the
c0aeac7I approved the mechanisms of — exactly what a "pin it" round should look like. I mutation-probed all three teardown paths and each probe bit exactly one test~ ♡Verdict: ✅ Looks good to me~
✅ What I liked~
if (_memberDrag == entityId) _memberDrag = null;clear atplayback_controller.dart:723→ exactly that test goes RED. "The poll owns the row again" is a pin now, not a hope~updateBackend's reset got its own directional test — my round-5 "equally unpinned" aside, answered without being asked to. Removing the_memberDrag = null;at:27→ "reconnecting drops a drag that was still in progress" goes RED. And I love that the test explains why: a stale drag must not shield a row on a new backend forever._memberDrag == snapshot.entityIddisjunct ofheld(:292). Dropping the disjunct → exactly that test RED. The defensive symmetry is now witnessed symmetry.flutter analyzeclean, and the reshaped test assertsvolumeCalls()length before mutating the entity — so it can't pass on a lucky double-send. Each probe reddened exactly one named test and nothing else. That's the discipline I've been sharpening my knives for across six rounds~💡 Little ideas (non-blocking)~
dart formatdrift on the touched lib files — carried from previous rounds, still honestly declined for tall-style-reasons with a CLAUDE.md citation. Someday, that dedicated commit~Automated review by Jibril · 2026-09-11
CI/CD: absent for head SHA (workflow targets the GitHub mirror) · Local checks:
flutter analyze0 issues, 100/100 tests pass, 3 mutation probes (release-clear / updateBackend-reset / lead-row disjunct), clone left pristine at4497cba