Talkback dock: source, nomination, and keying UI (M7) - #232
Merged
Conversation
…g (M7)
OWNER-APPROVED SPEC DEVIATION, recorded here because the spec says the
opposite: docs/superpowers/specs/2026-08-24-zoom-talkback-design.md locks
the keying surfaces to "Companion/Stream Deck, TCP/OSC control API, OBS
hotkey. **Not** the dock", and its Dock section says "Configuration and
tally only, by operator preference -- no talk button". The owner has since
asked for a dock that can actually be driven, so the dock keys. Nothing
else in that decision moved: identity is by display name, a key SELECTS a
channel nomination already created, and every refusal fails closed.
CLAUDE.md's talkback section carries the same note.
The group, top to bottom: talk-source picker (every OBS audio source, by
name, persisted as TalkbackSource) with the program-track warning under it
-- the deferred half of the leak guarantee, amber when the chosen source
feeds any mixer track, calm text naming the safe pattern otherwise; a
checkable roster list and a Nominate button; the budget outcome in full
(channels used of 16, who has a private channel, who has none, who is on
no channel at all, and the reason when the last attempt was refused); a
Latch toggle and one key button per confirmed nominee plus All; a tally
that follows the ENGINE's confirmed state ("LIVE to Sarah -- 2 of 3
present", a refusal reason, and the engine's own recovery hint echoed
rather than invented); and the dock's own refusals.
needs_renewal = false for dock keys. talkback-key.h's rule is that a
surface whose release is in-process and reliable does not need the
lost-release backstop: press/release are Qt signals on the same main
thread TalkbackController's QTimer runs evaluate()/key_off() on, with no
transport in between. Passing true would demand a heartbeat from a
UI-thread timer, which would close a genuinely held key on any ~1s OBS UI
stall. The one in-process way a release can still vanish -- QAbstractButton
clears `down` on an EnabledChange WITHOUT emitting released() -- is closed
twice over: the dock never disables a held button, and
talkback_dock_release_lost() closes a dock-owned PTT key whose button is no
longer down, from the existing 100ms tick (no new timer) and from the
widget's own state, so a stalled UI cannot false-close.
Decisions live in the new Qt/OBS-free src/talkback-dock-state.h and are
pinned by tests/talkback-dock-state-test.cpp (suite 67 -> 68, all green).
Enablement delegates to talkback_target_known_unprovisioned() -- the exact
predicate key_on() refuses on -- so a button shown as live can never be one
key_on() rejects for that reason; the test drives both against the same
plans. Three mutations proved the test bites: dropping the enablement rule,
dropping the unreachable line, and reporting live without engine
confirmation each fail it deterministically, all reverted.
Also here: TalkbackSessionStatus gains members_present/members_total (from
the engine's session_live stage line, as of the moment the key opened -- it
is not re-reported while the key is held) and `recover` (echoed from the
engine's own refusal line, never inferred); talkback_private_channel_names()
moves into talkback-plan.h so the dock label and the control API's
has_private_channel field cannot drift apart; prepare_shutdown() closes a
dock-owned key before its buttons go away.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0159FnPY9jGzKDF75y39WVxc
… close
M1 (Major, review of a58bac4). The latch toggle-OFF branch read the Latch
CHECKBOX at press time; talkback_key_released() read the mode captured at
the opening press. Two sources of truth for one mode, and this interleaving
made a live key un-closeable from the dock: latch on, key Sarah, uncheck
Latch, press Sarah to close -- the toggle-off was skipped, key_on() refused
"A talkback key is already open" as a red notice, and the following
released() bailed out because the captured mode still said latch. The
director stays LIVE to talent with the dock's only close affordance
answering with an error; the backstop exempts latch by definition and the
dead-man never fires while audio is flowing, so recovery meant Leave or the
control API.
There is now ONE record: TalkbackDockOpenKey {open, dock_owned, target,
latched}, where `latched` is the mode the key was OPENED with. All three
dock-side decisions read it -- talkback_dock_press_action() (the checkbox
now decides only what a NEW key opens as), talkback_dock_release_closes(),
talkback_dock_release_lost() -- and ZoomDock::dock_open_key() is the only
place the two members behind it are read. Mutation: reading `latch_selected`
instead of `open.latched` in the press decision fails the test with 2
deterministic failures, both naming the live-key case; reverted.
m3: a key held by ANOTHER surface now disables every button including the
open target's. key_on() refuses a second key unconditionally and the dock's
toggle-off cannot apply to a key it does not own, so that button could only
ever answer with an error. The stay-enabled exception is now scoped to
dock-owned keys, which is what makes its comment ("it is the one being
held") true.
m4: buttons refuse until a talk source is chosen, instead of offering the
first press every new operator makes and refusing it in the notice line.
m2: the source scan -- obs_enum_sources() (which holds
obs->data.sources_mutex and addrefs every source) plus
obs_get_source_by_name() -- was the first per-tick walk of libobs's source
list this dock has ever done, at 10Hz, whether or not anyone was looking.
Now gated on dock visibility AND 1Hz, with the rendered warning cached so
the label does not blank between scans; a source change resets the stamp so
the warning for the new pick is not a second late. Key state stays at 100ms.
This dock moved its feed-health sweep to 1Hz for exactly this reason.
Comment honesty, all three: n5 the timer count is four, not three (refresh,
health retry, recovery countdown, pending OAuth join). n6 "the ONE in-process
way a release can go missing" was wrong and the "closed twice over" framing
invited a future reader to delete the backstop -- QAbstractButton also drops
`down` on a non-popup focusOutEvent and on any setDown(false). The backstop
is cause-agnostic BY DESIGN (it asks the widget whether it is still down,
never why) and is load-bearing, not a second opinion; said that way in the
header, at the key_on() call site, and in CLAUDE.md. n7 the malformed-status
"consequence is bounded" claim reasoned only about button enablement -- the
same parse failure would have cleared m_talkback_dock_target and made the
dock forget it owns a live key, M1's end state by another route. Bound
fixed rather than restated: an unparseable status is UNKNOWN, the
ownership-clear is skipped for that tick, the dock's own record is the
fallback, and it logs once.
n8 (cosmetic row reordering) declined and recorded: roster order is the
operator's mental model of that list, and a ticked absent nominee keeps both
its name and its tick.
Suite unchanged at 68/68 -- the new latch/release/ownership cases went into
the existing CoreVideoTalkbackDockState target.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0159FnPY9jGzKDF75y39WVxc
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Flawfinder found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
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
2 participants
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.
The first in-OBS surface for talkback, in the Zoom Control dock: a talk-source picker with the program-track leak warning (the deferred M7 half of the leak guarantee), checkbox-roster nomination with the honest budget report (who has a private channel, who is uncovered, who is unreachable — named, never counted), PTT + latch key buttons per confirmed nominee plus All-talent, and a tally that follows engine-confirmed state only.
Keying from the dock is a recorded owner-approved deviation from the spec's surface list (spec locked keying to Companion/control API/hotkey).
Reviewed + one fix round: the open key is one record (
TalkbackDockOpenKey) read by all three dock-side decisions, so unchecking Latch can never strand a live key (the review's one Major — mutation-proved closed along with four others). Source scan gated on visibility at 1 Hz; buttons refuse with reasons when a key is held elsewhere or no source is chosen.Suite 68/68 (new
CoreVideoTalkbackDockStatetarget). Not yet driven in a live meeting — that's the next session's first click.🤖 Generated with Claude Code
https://claude.ai/code/session_0159FnPY9jGzKDF75y39WVxc