From c786958d13936ea83e12856f13f7b3c3f2447282 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Fri, 10 Jul 2026 20:17:13 -0500 Subject: docs: record the second review pass on the input-side spec I ran a second pass across four critical lenses: buildability, technical correctness, adversarial failure-modes, and internal consistency. The technical foundations held. I verified every load-bearing code and /proc/asound claim against the real tree. I recorded eight findings, two of them blocking. The mic-unmute remedy fails open on an unreadable graph, and a privacy event must fail closed. A Bluetooth or USB-non-ALSA mic classifies as no-mic-hardware because it never appears under /proc/asound. I also fixed three mechanical inconsistencies in place: a remedy miscount, and two stray OUTPUT/INPUT doctor-key labels the SPEAKERS/MICROPHONE rename missed. The spec stays DRAFT. The blocking findings gate it from READY. --- .../2026-07-10-audio-doctor-input-side-spec.org | 39 +++++++++++++++++++--- 1 file changed, 35 insertions(+), 4 deletions(-) (limited to 'docs/specs') diff --git a/docs/specs/2026-07-10-audio-doctor-input-side-spec.org b/docs/specs/2026-07-10-audio-doctor-input-side-spec.org index 9ed3be7..54f3fe4 100644 --- a/docs/specs/2026-07-10-audio-doctor-input-side-spec.org +++ b/docs/specs/2026-07-10-audio-doctor-input-side-spec.org @@ -239,7 +239,33 @@ Rejected in favor of one doctor per direction, which supplies the missing intent Rejected. The bottom tier should be the one thing in the doctor that cannot fail. -* Review findings [1/2] +* Review findings [1/10] + +Second review pass, 2026-07-10 (four critical lenses: buildability, technical correctness, adversarial failure-modes, internal consistency). The technical foundations checked out — every load-bearing code and =/proc/asound= claim verified against the real tree. These findings are what the passes surfaced beyond that. The mechanical inconsistencies they also found (a "two new remedies" miscount, two stray OUTPUT/INPUT doctor-key labels) are already fixed in place. + +** TODO Mic-unmute must fail closed on an unreadable graph :blocking: +The stream-active guard abstains and proceeds when =pw-dump= cannot read the graph. That is right for the *output* remedies, whose whole job is repairing a dark graph. It is wrong for the mic-unmute remedy (8), because unmuting is a privacy event, not an availability one: with the graph unreadable the guard cannot see a running capture stream, so FIX would unmute a live mic mid-recording with no second press and no warning. The mic-unmute remedy must fail *closed* on an unreadable graph — refuse, or fall back to =pactl list source-outputs= — never abstain-and-proceed. Every other finding here produces a wrong verdict; this one produces a wrong action. Folds into the open capture-guard decision. (blocking) + +** TODO A non-ALSA microphone classifies as no-mic-hardware :blocking: +A Bluetooth or USB-non-ALSA mic never appears under =/proc/asound/card*=, because bluez runs through PipeWire, not ALSA. The =no-mic-hardware= verdict is gated only on "kernel capture set empty", with no graph clause, so a working BT-headset mic (a real non-monitor source in the graph) reads as "No microphone is attached." The input ladder must consult the graph first: a working non-monitor source in the graph is =healthy= (or routes to the semantic rules) and skips the kernel tier entirely. =no-mic-hardware= fires only when the kernel set is empty *and* the graph has no non-monitor source; =mic-unrecognized= only when the kernel set is non-empty *and* the graph has none. This also resolves the ladder-order contradiction below. Folds into the open input-verdicts decision. (blocking) + +** TODO The input ladder's evaluation order is stated two ways :important: +The "For the implementer" prose lists the kernel-tier rules first (no-mic-hardware, mic-unrecognized), then the semantic rules. The failure table orders the semantic-input rows (tier 4) above the kernel rows (tier 5) and says lower rows are reached only after upper tiers answer clean. Those are opposite orders for the same four rules. The finding above settles it: graph-source presence is the top gate, so define one order and make both the prose and the table match it. + +** TODO The probe does not yet surface source mute, volume, or a monitor flag :important: +=probe_semantic= produces =default_sink_muted= / =default_sink_volume= but no source equivalents, and neither the graph nor the semantic source dicts carry a monitor flag (=pactl._device= strips =device.class=). The =mic-silenced= rule reads source mute/volume the context never provides, and the "non-monitor hardware source" test cannot be computed. Phase 0 must extend =probe_semantic= to surface the source fields and tag sources monitor-vs-hardware, and the =mic-unrecognized= rule must read the =pw-dump= source population (=Audio/Source=, monitor-excluding), not the =pactl include_monitors=True= one — reading the monitor-inclusive list would mean a monitor is always present and the rule never fires. Neither change is scoped by an open decision; both would stall Phase 0/1. + +** TODO The direction must thread through findings() and doctor(), not only classify() :important: +The spec says =classify(ctx)= becomes =classify(ctx, side)= but never that =findings(ctx)= gains =side= too, though the wall shows side-specific rows. =doctor.doctor()= is the sole caller and invokes both =classify= and =findings= twice each (initial and re-probe), plus =final_findings= / =final_verdict=; it must take =side= and thread it to all of them, and =cli.cmd_doctor= must pass the resolved side. Name this in the phase text so it is not left for the implementer to rediscover. + +** TODO Remedy 6 restarts WirePlumber unguarded, mid-call :important: +=mic-unrecognized='s remedy (6, restart WirePlumber) is in neither guard set, so a user on a call can press FIX and reload the device profiles with no guard and no second-press consent. A profile reload can re-select the default sink and drop the call's routing. Decide whether remedy 6 is guarded — against streams of *both* directions, since a WirePlumber restart touches the whole graph — as part of the open capture-guard decision. + +** TODO A stale armed=true PTT state can mask a genuinely muted mic :important: +The safe-degrade argument covers only an unreadable or missing PTT state file (=read_state= returns disarmed). It does not cover =armed=true= left stale — Hyprland restarted and dropped the binds, or the mic was muted by the CONTROLS toggle for an unrelated reason while PTT happened to be armed. =read_state= cannot detect staleness, so the doctor reports "healthy, PTT armed" on a genuinely dead mic, the exact failure this feature exists to fix. At minimum, name the limitation; better, have the informational row say the mic is muted *and* that PTT is armed, so the user is not told the mic is simply fine. + +** TODO Minor build-notes to fold in before Phase 3 :minor: +Three smaller items the passes raised. The =healthy= verdict text is output-worded ("the default output is present and audible") and needs side-aware wording for a clean =MICROPHONE= run. The two keys must clear =doctor_running= on the worker's error path so a raised worker cannot wedge both keys insensitive for the session — the shipped =bg= / =doctor_abort= pattern already does this, so it is a spec-explicitness note, not a code gap. And a USB mic pressed during its ~1s enumeration window (kernel sees the card before WirePlumber creates the source) fires a transient =mic-unrecognized=; self-correcting, worth a line acknowledging the race. ** TODO Open Decisions still gate implementation :blocking: The workflow treats unresolved decisions as implementation blockers, because a builder would be accepting product and safety tradeoffs mid-build. Partly addressed as of 2026-07-10: Craig closed the push-to-talk and kernel-tier decisions, and the CLI-flag decision closed with his direction, so the cookie is now =[6/9]=. Three decisions remain open — the direction-aware capture guard, the input verdict and remedy table, and the doctor-key naming and cross-panel style. Close or explicitly risk-accept each before implementation starts. (blocking) @@ -262,7 +288,7 @@ Consequences: the ambiguity is resolved by the user without a dialog, and the ch CLOSED: [2026-07-10 Fri] Context: the wall holds one verdict, and a run clears the wall when it begins. -Decision: we will allow only one doctor run in flight. OUTPUT must finish before INPUT can be pressed, and both keys go insensitive for the duration. +Decision: we will allow only one doctor run in flight. SPEAKERS must finish before MICROPHONE can be pressed, and both keys go insensitive for the duration. Consequences: the wall is never a mix of two runs, and "which verdict is this" never needs asking. Harder: nothing, since the model already clears on begin and locks on =doctor_running=. The keys simply join the lock. @@ -309,7 +335,7 @@ Owner: Craig. By: before Phase 2 lands. ** TODO The input verdicts and their remedies Context: the input side needs its own verdict vocabulary and its own remedy table entries. -Decision (proposed): four new verdicts, and two new remedies. +Decision (proposed): four new verdicts, and three new remedies. | Verdict | Means | Remedy | Tier | |---------------------+------------------------------------------+-------------------------------------------+---------| @@ -379,7 +405,7 @@ And amend the parent spec's history to record that its DOCTOR-key decision was s - A muted mic with PTT disarmed classifies as =mic-silenced= and FIX unmutes it. - FIX on the mic while something is recording is refused, and a second press proceeds. - Restarting a playback remedy while only a microphone is recording is no longer refused. -- Pressing OUTPUT while INPUT is running does nothing. +- Pressing SPEAKERS while MICROPHONE is running does nothing. - =audio doctor= with no flag behaves exactly as it does today. * Readiness dimensions @@ -409,6 +435,11 @@ This spec supersedes a decision from a spec that reached IMPLEMENTED the same da * Review and iteration history +** 2026-07-10 Fri @ 18:52:07 -0500 — Craig Jennings — Reviewer +What changed or was recommended: a second review pass across four critical lenses — buildability, technical correctness, adversarial failure-modes, internal consistency. Verified every load-bearing code and =/proc/asound= claim against the real tree; all held. Recorded eight new findings (two blocking: mic-unmute failing open on an unreadable graph, and a non-ALSA mic mis-classifying as no-mic-hardware) and fixed three mechanical inconsistencies in place (a remedy miscount and two stray OUTPUT/INPUT doctor-key labels). +Why: the design was sound at the tier level, but it had two safety and correctness holes that only surface on real hardware (a webcam or Bluetooth mic, an unreadable graph), plus three build gaps that sit outside the open decisions and would stall Phase 0/1. +Artifacts: [[*Review findings][Review findings]]; code read across =~/.dotfiles/audio/src/audio/= (=diag.py=, =classify.py=, =repair.py=, =doctor.py=, =ptt.py=, =pactl.py=); live =/proc/asound= survey on ratio. + ** 2026-07-10 Fri @ 10:55:19 -0500 — Codex — Reviewer What changed or was recommended: applied the spec-review workflow and recorded two blocking findings: open Decisions still gate implementation, and kernel-capture-to-graph-source matching is undefined. Why: the current implementation confirms the microphone gap and output-only classifier, but these unresolved choices would force the builder to decide safety and classification behavior during implementation. -- cgit v1.2.3