A controls-free pill for push-to-talk - #3
Open
samartomar wants to merge 3 commits into
Open
Conversation
A hold has no decisions in it — press, speak, release, and the words are pasted — and every control on the pill row is a decision it never asks for. So the row drops to two frames inside one 90 px box: at rest the focused app's name and the mic glyph, and while the chord is held the level bars and nothing else. Off by default, one checkbutton under Settings, offered only while the chord is the hold gesture. It is a view and not a mode, which is what makes "no impact on the existing behaviour" structural rather than a promise. flow/session.py is untouched and emits exactly the events it emits without this; Pill._pump_events draws fewer of them. There is no branch in a session route to get wrong because there is no branch in a session route at all, and tests/test_mic.py asserts that twice — against the source, and against a scripted drain whose session-side calls are identical with the view on and off. The hard part was the note. With no panels there is nowhere for a refine's commentary, an unreachable CLI or an unverified-CLI line to land, and swallowing one would make this the only place in Flow where a refusal is silent. Because it is a view it can grow back to the full pill for that moment and shrink again with no state change: mic_view is a property that reads whether a surface is up, so the grow-back is Bubble.note calling surface on a hidden bubble and the shrink is that surface going away. Placement is free here and only here. PLACES has two entries because the pill anchors a stack that must fit a 400/480/580 px panel; this row anchors nothing, so it persists an (x, y) — dragged into place, saved on the release, re-clamped against the work area on the way out because the answer is per monitor and the profile is not. Two things a rendered run found that the fake canvas could not, both fixed: - The sent card opened the panel on every single utterance. A release ends in _pump_talk calling _send, and _send put a receipt on screen for words that had already gone into the other window — through the one door that is not an event, so the routing gate never saw it. Gated. The failure branch is not: a paste that failed or could not be guaranteed still opens the panel and still flashes, because it is the one thing here that cannot be read off the other window. - app_label cuts at APP_NAME_CHARS characters, and characters are not pixels in a proportional font: ten of them are 25 px of Code and 69 px of WindowsTe…, printed straight through the mic glyph. Cut by measurement now (Pill._fit_note, memoised because _draw may not build a font per frame). This was first written off as an overrun the full row never shows, on the reasoning that its glyph is further away. A screenshot settled it — 69 px from x=10 against an arc starting at 61 — so _draw's own label goes through the same cut at APP_SLOT_W. That half is a fix for everyone, view or not. scripts/shots.py now walks both frames, the longest name, the grow-back and the shrink, so this stops depending on anyone remembering to look. It needed a chord on its fake hotkeys, without which the walk was silently missing the Chord row too. An exception in the frame pump leaves the row painted at the last width under a window that has already been resized — found while photographing this, pre-existing, and filed in NEEDS_YOU.md rather than fixed, because the choice between the two candidate fixes is a contract call about the frame loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12bcb280f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # sets both), and reaching through a window meant this could be asked before | ||
| # there was one to ask. | ||
| return BUBBLE_W | ||
| return MIC_W if self.mic_view else BUBBLE_W |
There was a problem hiding this comment.
When the mic view is dragged near the right edge and a note temporarily opens the full panel, this width switch makes _sync_shell() clamp self.x leftward so the 400 px panel fits. After the note closes and the width returns to MIC_W, _sync_shell() preserves that clamped coordinate instead of restoring _mic_at, leaving the pill permanently displaced for the rest of the session. Reapply the saved mic coordinate when the temporary surface closes.
Useful? React with 👍 / 👎.
| self.apps = take("apps", lambda v, _d: _apps(v), {}) | ||
| self.panel = take("panel", _text, PANEL_DEFAULT) | ||
| self.place = take("place", _text, PLACE_DEFAULT) | ||
| self.mic = take("mic", lambda v, _d: bool(v), False) |
There was a problem hiding this comment.
If a profile is hand-edited or corrupted to contain "mic": "false" (or any other nonempty non-boolean value), bool(v) loads it as True without recording a fault, so Flow unexpectedly starts in the controls-free view. The existing boolean fields deliberately use _flag to avoid exactly this truthiness conversion; this field should use the same strict validation.
Useful? React with 👍 / 👎.
…opens nothing Both found by running the app rather than the suite, which had asserted the shrink by calling hide() itself and so proved only that hiding works. A note that grew the full pill back never left. Nothing hides a surfaced note: on the full row it lands on a panel that was already up and the next draft clears it, and under this view there is no next draft. So one line from a settings row — clicking "Push to talk" in the Chord cascade — left the pill 400 px wide for as long as Flow ran, measured at twelve seconds and still up. Bubble.tick_note gives the row back after MIC_NOTE_SEC, stamped only where this view surfaced it and dropped the moment the panel has a second reason to be there, so every note on the full pill is untouched. And progress opened a panel. tick_activity surfaces the bubble for a wait with nothing else on screen, which is right on the full row — the invisible states are the ones with no draft to hang a note on — and wrong here: the model load opened a 400 px panel on every launch and held it for the eight seconds the load takes. Measured from a cold start: 90x34 at 0.76 s, 400x98 at 1.07 s, back to 90x34 at 9.52 s, which is the startup reported as "it opens as a big window then it shifts to mic". The grow-back is for a refusal; an activity is the opposite, and the mic glyph is already carrying that state in its colour. It declines to open one rather than closing one: a wait already up is left alone, and a note arriving during a load still surfaces. Push-to-talk itself was checked and is not affected. Driven through the real _talk_start/_talk_end on a real Pill it holds, draws the meter frame, ends, and leaves no panel behind. A synthetic ctrl+win did not reach the chord hook in either configuration — with the view on and with it off — so that harness proves nothing either way, and the two configurations behave identically. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_invoke read stderr alone on a non-zero exit, on the stream discipline this module documents and measures: the answer on stdout, the banner and errors on stderr. That holds while a CLI is working. It does not hold when one refuses. Measured against claude 2.1.248 on 2026-09-03: `claude --safe-mode -p` exits 1 with "Not logged in - Please run /login" on stdout and nothing at all on stderr. So an ask with a logged-out claude reached the card as `ask failed (claude exited 1: ; then kiro-cli exited 1: Not logged in. Set the KIRO_API_KEY environment variable ...)` — the only CLI that explained itself was the fallback, and the line the user could act on for the CLI they actually run was thrown away between the colon and the semicolon. A refusal that prints an empty reason is worse than a silent one: it looks like it said something, which is P2 failing where it is hardest to notice. _why takes the first non-blank line of stderr, and stdout only when stderr had none. stderr keeps first place for two reasons rather than one: it is where a CLI that separates its streams puts the error, and a CLI can exit non-zero having already written part of an answer to stdout — repeating the head of that as the diagnosis would be a fabrication, which is worse than the blank it replaces. Bounded to one line and _WHY_CHARS, because some CLIs print a stack trace and a panel is not a terminal. Verified against the real CLI: the note now reads `claude exited 1: Not logged in - Please run /login`. Co-Authored-By: Claude Opus 5 <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.
A hold has no decisions in it — press, speak, release, and the words are pasted — and every control on the pill row is a decision it never asks for. So the row drops to two frames inside one 90 px box: at rest the focused app's name and the mic glyph, and while the chord is held the level bars and nothing else. Off by default, one checkbutton under Settings, offered only while the chord is the hold gesture.
A view, not a mode
flow/session.pyis untouched and emits exactly the events it emits without this;Pill._pump_eventsdraws fewer of them. That is what makes "no impact on the existing behaviour" structural rather than a promise — there is no branch in a session route to get wrong because there is no branch in a session route at all.tests/test_mic.pyasserts it twice: against the source, and against a scripted drain whose session-side calls are identical with the view on and off.The paste is the existing paste-on-release. Optimistic paste stays rejected — a paste cannot be taken back in a terminal.
The hard part: a refusal is never silent
With no panels there is nowhere for a refine's commentary, an unreachable CLI or an unverified-CLI line to land. Because this is a view it can grow back to the full pill for that moment and shrink again with no state change:
mic_viewis a property that reads whether a surface is up, so the grow-back isBubble.notecallingsurfaceon a hidden bubble, and the shrink is that surface going away. Nothing is entered, so nothing has to be unwound.Placement
PLACEShas two entries because the pill anchors a stack that must fit a 400/480/580 px panel. This row anchors nothing, so it persists an(x, y)— dragged into place, saved on the release, re-clamped against the work area on the way out, because the answer is per monitor and the profile is not.Two bugs a rendered run found that the fake canvas could not
The sent card opened the panel on every single utterance. A release ends in
_pump_talkcalling_send, and_sendput a receipt on screen for words that had already gone into the other window — through the one door that is not an event, so the routing gate never saw it. Gated. The failure branch is not: a paste that failed or could not be guaranteed still opens the panel and still flashes, because it is the one thing here that cannot be read off the other window.WindowsTe…printed straight through the mic glyph.app_labelcuts atAPP_NAME_CHARScharacters, and characters are not pixels in a proportional font — ten of them are 25 px ofCodeand 69 px ofWindowsTe…. Cut by measurement now (Pill._fit_note, memoised because_drawmay not build a font per frame). This was first written off as an overrun the full row never shows; a screenshot settled it — 69 px from x=10 against an arc starting at 61 — so_draw's own label goes through the same cut atAPP_SLOT_W. That half is a fix for everyone, view or not.Also
scripts/shots.pynow walks both frames, the longest name, the grow-back and the shrink, so reviewing this stops depending on anyone remembering to look. It needed achordon its fake hotkeys, without which the walk was silently missing the Chord row too.An exception in the frame pump leaves the row painted at the last width under a window that has already been resized — found while photographing this, pre-existing, and filed in
NEEDS_YOU.mdrather than fixed: the choice between the two candidate fixes is a contract call about the frame loop.Tests
67 tests in
tests/test_mic.py. Full suite 2315, with the six pre-existingtest_voiceerrors (sounddevicenot installed on this machine) unchanged.🤖 Generated with Claude Code