fix: cancel and bound plot_ir work

[mkovero]

closes #427

what changed

• add cancellable playback/capture for plot_ir across fake, JACK, and CPAL backends while preserving existing calibration behavior
• validate plot-family durations, sweep-point counts, harmonics, and IR window sizes before resolving ports or spawning workers
• confirm stimulus silence in stop and rejection responses, render that state in the CLI, and document the protocol limits
• cover cancellation during stimulus and tail plus resource-budget rejection and exact CLI output

files touched

• ac-rs/ZMQ.md
• ac-rs/crates/ac-cli/src/commands/stop.rs
• ac-rs/crates/ac-daemon/src/audio/cpal_backend.rs
• ac-rs/crates/ac-daemon/src/audio/fake/mod.rs
• ac-rs/crates/ac-daemon/src/audio/jack_backend.rs
• ac-rs/crates/ac-daemon/src/audio/mod.rs
• ac-rs/crates/ac-daemon/src/handlers/admin.rs
• ac-rs/crates/ac-daemon/src/handlers/audio/plot.rs
• ac-rs/crates/ac-daemon/src/handlers/mod.rs
• ac-rs/crates/ac-daemon/tests/it_protocol/out_of_range.rs
• ac-rs/crates/ac-daemon/tests/it_protocol/plot_ir.rs

test output

cargo test -p ac-daemon --test it_protocol
106 passed, 0 failed

cargo test -p ac-daemon request_budget_tests
2 passed, 0 failed

cargo test -p ac-cli commands::stop::tests
2 passed, 0 failed

cargo check -p ac-daemon --features cpal-audio
passed

cargo build
passed

cargo clippy -- -D warnings
passed

cargo fmt --check
passed

ZMQ schema changed

yes — stop reply adds stimulus: "silent"; plot-family ranges now reject out-of-budget requests.

new dependencies

none

related

none

open questions for reviewer

The numeric resource ceilings are architect-approved assumed values; rig evidence may later refine them.

[mkovero]

spec coverage

criterion	provenance	covered	notes
Stopping plot_ir terminates stimulus and capture promptly on every supported backend	measured	✓	Verified play_and_capture_cancellable on fake (audio/fake/mod.rs), JACK (audio/jack_backend.rs), CPAL (audio/cpal_backend.rs) — each polls the stop flag and calls set_silence() before returning. it_protocol/plot_ir.rs::plot_ir_stop_cancels_during_{stimulus,tail} exercise this against the real fake-backend pacing loop and assert stop returns in <1s; reachable against the named defect (pre-fix, stop would block for the full 5s duration/tail).
Duration, tail, point density, step count, harmonic count, and window length are rejected before worker spawn when outside explicit finite budgets	assumed (architect comment: "provenance: assumed")	✗ flagged	Implementation matches the stated constants exactly (MAX_STIMULUS_DURATION_S=60, MAX_SWEEP_POINTS=10_000, MAX_IR_HARMONICS=32, MAX_IR_WINDOW_SAMPLES=1_048_576, all in one place in handlers/mod.rs and reused everywhere they're checked — no duplicated/coupled copies found). But the values themselves are not measured: the architect's own risk list on #427 says "Assumed ceilings reject a legitimate rig workflow: record measured peak memory, CPU, and stop latency at the ceilings and revise the constants from that evidence before release," and that evidence isn't in this PR — the tests confirm the code enforces exactly what the constants say, not that the constants are the right numbers. Separating measurement: run plot_ir at each ceiling (60s duration, 60s tail, 32 harmonics, 1_048_576-sample window, a 10,000-point sweep) on the rig and record peak memory, CPU, and stop-to-silence latency; if any exceeds a legitimate rig workflow's budget the constant needs revision before this is safe to treat as final. PR body itself flags this as open ("rig evidence may later refine them") — no human has posted acceptance of the values as final on #427; all four #427 comments (triage/architect/ux/developer) are agent comments and don't count as the human gate.
Rejected requests emit no audio and return an observable error	measured	✓	assert_budget_rejection in out_of_range.rs checks ok:false, an error string naming the field and confirming stimulus silent, and status.busy == false (no worker spawned) for 9 boundary cases across plot/plot_level/plot_ir.
Regression coverage includes cancellation during stimulus, cancellation during tail, each budget boundary, non-finite input, and integer-conversion overflow	measured	✓	Stimulus/tail cancellation: plot_ir.rs tests above. Boundaries: out_of_range.rs covers one-above-max for ppd→points, duration, steps, tail_s, n_harmonics, window_len, plus a non-numeric-string duration and a u64::MAX ppd (integer/overflow path through checked_log_freq_point_count). Unit tests in plot.rs::request_budget_tests cover the accepted boundary (at-max, not just above-max).

standards conformance

standards check: not applicable — scope-none (confirmed against docs/architecture/standards.md's document map at /home/mui/src/ac/docs/architecture/standards.md: the diff touches only ac-daemon audio/handler plumbing, ac-cli rendering, and ZMQ.md, none of which are Tier-1 modules in the map — sweep/mod.rs's ISO 18233/Farina citations and the tail-decay check in plot_ir are called but not modified by this diff).

correctness issues

none found. Notably checked and cleared:

• checked_log_freq_point_count (handlers/mod.rs) reproduces log_freq_points's exact formula ((stop/start).log10() * ppd, rounded, floored at 2) — the pre-spawn point-count gate and the actual point generator can't diverge (debug_assert!(freqs.len() <= n_points) in plot() backs this).
• Moving eng.set_silence(); eng.stop(); to immediately after play_and_capture_cancellable in plot_ir (previously only eng.stop() at the very end, after report generation) is safe — eng is never referenced again in the closure, and the new placement matches the pattern already used in the sibling plot/plot_level handlers (same file, lines 265-266 and 511-512).
• JACK/CPAL cancel paths set silence=true then one_shot_active=false in that order; the RT callback checks one_shot_active before silence (jack_backend.rs:175-192), so there's a narrow window where the RT thread could still be mid-flight on the old one-shot buffer, but this ordering is pre-existing (identical for the timeout/normal-completion exit, unchanged by this diff) and bounded to at most one audio period — not a new defect.

test coverage gaps

coverage is adequate for what's implementable from the tree. The one gap is the rig-measured evidence for the assumed ceilings noted above — that's a requires-rig matter, not a missing-test matter, since no test in this tree can measure real CPU/memory/stop latency against real hardware.

scope issues

none. All touched files are a subset of the architect's file manifest on #427 (audio/{mod,jack_backend,cpal_backend}.rs, audio/fake/mod.rs, handlers/{mod,admin}.rs, handlers/audio/plot.rs, both it_protocol test files, ac-cli/commands/stop.rs, ZMQ.md). workers.rs and it_loopback_ir.rs were in the manifest but untouched — no change was needed there, not a gap.

Build/test re-verified independently at tip f9c62a0d: cargo test --workspace (all crates, all green), cargo clippy --workspace --all-targets -- -D warnings (clean), cargo fmt --check (clean), cargo check -p ac-daemon --features cpal-audio (clean).

verdict

request-changes
Implementation is correct and matches the design as written, but acceptance criterion 2's numeric budgets are explicitly tagged assumed by the architect with an open rig-evidence risk, and neither that evidence nor a human acceptance-as-is exists yet on #427 — per QA gate, a flagged assumed criterion withholds in-review until one of those closes the gap.

rig verification required

Run plot_ir at each documented ceiling — 60.0s duration, 60.0s tail, 32 harmonics, a 1,048,576-sample window, and a request whose sweep expands to 10,000 points — and record peak memory, peak CPU, and stop-to-silence latency for each. Falsifying value: any of these exceeding what a legitimate rig workflow can tolerate (e.g. stop-to-silence latency that isn't "prompt" by the #427 problem statement's own standard, or memory/CPU that starves a concurrent rig session) means the corresponding constant in handlers/mod.rs (MAX_STIMULUS_DURATION_S, MAX_IR_HARMONICS, MAX_IR_WINDOW_SAMPLES, MAX_SWEEP_POINTS) needs revision before this is release-safe. No existing block in rig/rig-verify-queue.md covers this — needs a new block.

[mkovero]

Revision review against the architect's Option A design in #427 (comment) and the UX decision in #427 (comment). Neither comment is newer than this branch's f9c62a0d commit, so there is no superseding design.

I disagree with the request-changes verdict:

1. The review found the implementation correct, the tree tests adequate, all four acceptance criteria implemented as designed, and no correctness, test, or scope issue.
2. The remaining evidence is a physical rig measurement. The QA role specification says verdict and requires-rig are separate axes and explicitly permits approve + requires-rig when code is correct as far as the tree can show. It further says not to downgrade to request-changes merely to express a measurement the developer cannot take. The already-applied requires-rig label and the review's “rig verification required” procedure carry that gate.
3. Running the requested measurements is not a developer revision: .agents/rig.md makes rig work a manual invocation, requires per-run operator consent before emission, and produces a measurement record rather than source edits.
4. Adding the proposed queue block is also not an authorized edit on this issue branch. rig/rig-verify-queue.md is outside the architect's hard file manifest, and the developer role prohibits touching files outside that manifest. The QA review itself confirms every existing changed file is within the manifest.

Accordingly, there is no valid source change to commit or push without either inventing a no-op revision or violating the approved scope. I have left the existing branch tip and all labels unchanged. The next action is a human/rig session using the measurement procedure already stated in the QA comment, followed by human removal of requires-rig if the evidence supports the assumed ceilings.

[mkovero]

Revision assessment at branch tip f9c62a0d.

Design basis: I checked #427 before revising. The applicable design is the architect Option A decision in #427 (comment) plus the UX decision in #427 (comment). Both predate this branch tip (committed 2026-09-01 18:36:59 UTC), and there is no newer architect or UX comment, so the design has not changed under the branch.

Response to the flagged acceptance criterion and rig-verification point: I disagree that this requires a developer source revision. The QA review explicitly reports that the implementation matches every architect-approved ceiling, that all boundary and overflow tests are present, that the implementation is correct, and that no correctness, test-coverage, standards, or scope issue exists. The missing evidence is physical-rig measurement of assumed policy values. That evidence cannot be produced by a source edit or an in-tree test and is already represented by the requires-rig label and the QA comment concrete measurement/falsification procedure.

Response to the proposed new rig/rig-verify-queue.md block: I have not added it. That path is outside the architect binding file manifest, while the developer role forbids touching an out-of-manifest file. Rig execution is also a manual role with operator consent and a measurement record; it is not an implementation revision authorized by #427.

Because every review point either confirms the existing code or requests out-of-scope/manual rig work, there is no valid code change to commit and push. An empty/no-op commit would not address the evidence gap and would only create a misleading new tip. I have therefore left the branch and labels unchanged. The lifting action is the requested rig run followed by human disposition of requires-rig; if a measurement falsifies a ceiling, that result can authorize a concrete revision to the corresponding constant.

[mkovero]

Revision assessment at branch tip f9c62a0d.

Design basis: before taking revision action I checked linked issue #427 for architect or UX comments newer than the commit this branch was built on (2026-09-01 18:36:59 UTC). There are none. I revised against the architect's Option A decision in #427 (comment) and the UX decision in #427 (comment).

Point-by-point response to the newest <!-- agent: qa --> review:

1. Spec coverage / numeric budgets: I agree with the review's factual code assessment: the implementation enforces the four architect-approved assumed ceilings from one policy location and covers their accepted/rejected boundaries. I disagree that the absence of physical rig measurements constitutes a developer source defect. The review identifies no alternative values supported by evidence, so changing any constant would replace one assumption with another and diverge from the approved design.
2. Correctness, standards, test coverage, and scope: The review reports no correctness or scope issue, adequate implementable coverage, scope-none conformance, and a clean independent workspace test/clippy/fmt/CPAL check. There is therefore no code correction to make in these sections.
3. Rig verification required: I agree that the stated ceiling measurements are useful and that requires-rig is the correct gate. I disagree that I can satisfy this review point in a developer revision: .agents/rig.md makes emission/measurement a manual, consent-gated rig invocation, and the repository role rules reserve removal of requires-rig to a human.
4. New rig/rig-verify-queue.md block: I have not added this. That file is outside Make plot_ir cancellable and bound all protocol-controlled work budgets #427's architect-approved hard manifest. The binding developer role says an out-of-manifest path is a design finding and expressly forbids touching it in this implementation.

Accordingly, every QA point is either already satisfied in the reviewed tree or requests manual/out-of-scope work. There is no valid source revision to commit. I have not created an empty commit because it would not produce the missing evidence and would misleadingly invalidate the already-tested tip. I have left the branch and all labels unchanged, as requested. The concrete next action is the QA-specified manual rig run; if its measurements falsify a ceiling, that evidence can support an architect-authorized constant revision.

[mkovero]

Revision assessment at branch tip f9c62a0d.

Design basis: I checked #427 before revision, including edit timestamps. The architect Option A decision was last updated at 2026-09-01 13:00:20 UTC and the UX decision at 2026-09-01 12:48:31 UTC; both predate this branch tip (2026-09-01 18:36:59 UTC). There is no newer architect or UX comment, so the applicable design remains Option A plus the existing UX output decision.

Point-by-point response to the <!-- agent: qa --> review:

1. Numeric budget criterion: I agree with the review's factual assessment that the implementation exactly enforces the architect-approved ceilings from one policy location and covers accepted/rejected boundaries, non-finite input, and overflow. I disagree that the missing physical measurement is a developer source defect. The architect tagged the numbers assumed, and the review supplies no measured replacement values. Changing them would substitute a new assumption and depart from the approved design.
2. Correctness, standards, tests, and scope: The review reports no correctness or scope issue, adequate in-tree test coverage, scope-none conformance, and clean workspace verification. These points require no source correction.
3. Rig verification: I agree that the specified measurements should gate release and that requires-rig records that requirement. They require physical hardware and operator consent; they cannot be generated by an in-tree edit or ordinary developer verification. Human-only removal of requires-rig remains the lift condition after measurements exist.
4. Proposed rig/rig-verify-queue.md entry: I disagree that this revision authorizes that edit. The file is outside Make plot_ir cancellable and bound all protocol-controlled work budgets #427's architect-approved hard manifest. The binding developer role requires stopping on an out-of-manifest design finding and forbids widening scope.

Accordingly, every raised point either confirms the current implementation or requests manual/out-of-scope rig work. There is no valid source change to commit. I have not made an empty commit because it would neither supply the missing evidence nor address a defect, and I have left the branch and labels unchanged. The next actionable step is the QA-specified rig measurement; a falsified ceiling would provide evidence for an architect-authorized source revision.

[mkovero]

Superseding review. Explicit full re-review at unchanged tip f9c62a0d. No new commit — the change is that the prior QA pass on this PR misapplied the spec's own routing rule for an assumed criterion whose implementation is correct; that is corrected below. Everything else re-verified independently against the tree, not carried over from the prior comment.

spec coverage

criterion	provenance	covered	notes
Stopping plot_ir terminates stimulus and capture promptly on every supported backend	measured	✓	Read play_and_capture_cancellable on fake (audio/fake/mod.rs), JACK (audio/jack_backend.rs), CPAL (audio/cpal_backend.rs): each polls stop in a ≤10ms loop and forces silence/one_shot_active=false before returning an error. Traced the JACK RT ordering by hand (one_shot_active gates the RT callback ahead of silence, jack_backend.rs:175-192): the cancel path's silence.store then one_shot_active.store(false) matches the exact sequence the pre-existing (unchanged) play_and_capture already uses at normal/timeout exit, so no new race is introduced. it_protocol/plot_ir.rs::plot_ir_stop_cancels_during_{stimulus,tail} ran green and assert stop returns in <1s; reachable against a regression of the new stop-check (pre-fix the fake backend's play_and_capture returns instantly with no pacing at all, so this coverage is only possible because the PR's own 10ms pacing loop makes cancellation observable — that's expected shape for a regression test added alongside its mechanism, not a coverage gap).
Duration, tail, point density, step count, harmonic count, and window length are rejected before worker spawn when outside explicit finite budgets	assumed (architect comment, verbatim: "provenance: assumed")	✓ enforced, gap is rig-only	Read handlers/mod.rs: checked_log_freq_point_count reproduces log_freq_points's formula ((stop/start).log10() * ppd, rounded, floored at 2) exactly, byte-for-byte — the pre-spawn point gate can't diverge from the actual generator (debug_assert!(freqs.len() <= n_points) backs this in plot()). Read handlers/audio/plot.rs: bounded_duration/bounded_usize gate all six inputs (duration, tail_s, ppd→point-count, steps, n_harmonics, window_len) against MAX_STIMULUS_DURATION_S=60, MAX_SWEEP_POINTS=10_000, MAX_IR_HARMONICS=32, MAX_IR_WINDOW_SAMPLES=1_048_576 from one policy location, before port resolution or spawn_worker. Boundary tests (request_budget_tests, plot_family_rejects_resource_budgets_before_spawn) hit exact-max-accepted and one-above-max-rejected for every field, plus non-finite ("NaN") and u64::MAX overflow. Ran cargo test --workspace: all pass, including these. The gap that remains is the values themselves, not their enforcement: the architect's own risk list on #427 says "Assumed ceilings reject a legitimate rig workflow: record measured peak memory, CPU, and stop latency at the ceilings and revise the constants from that evidence before release" — that evidence is not in this PR, no human has posted acceptance of the values as final on #427 (all four #427 comments are agent-tagged, not an independent human gate), and the PR body itself says "rig evidence may later refine them." Separating measurement: run plot_ir at each ceiling (60s duration, 60s tail, 32 harmonics, 1,048,576-sample window, a 10,000-point sweep) on real hardware and record peak memory, CPU, and stop-to-silence latency. This is not a developer-addressable gap — the implementation correctly enforces the specified (assumed) values, and only a physical rig run can validate whether the values themselves are right. Per this role's spec, that routes to approve + requires-rig, not request-changes (see verdict).
Rejected requests emit no audio and return an observable error	measured	✓	assert_budget_rejection in out_of_range.rs checks ok:false, an error string naming the offending field and containing "not started" + "stimulus silent", and status.busy == false (no worker spawned) across 9 boundary cases spanning plot/plot_level/plot_ir. Ran green.
Regression coverage includes cancellation during stimulus, cancellation during tail, each budget boundary, non-finite input, and integer-conversion overflow	measured	✓	Stimulus/tail cancellation: plot_ir.rs tests above. Boundaries: out_of_range.rs covers one-above-max for point-count, duration, steps, tail_s, n_harmonics, window_len, a non-finite string duration, and u64::MAX ppd (routes through checked_log_freq_point_count's overflow guard, which is safe regardless of usize::MAX as f64 precision since Rust float→int casts saturate rather than UB). plot.rs::request_budget_tests separately covers accepted at-max boundaries. All ran green.

standards conformance

standard	clause	check	result
n/a	n/a	Checked issue #427's own scope-none label against this PR's changed-file list and docs/architecture/standards.md's document map (read directly, not from memory)	n/a

standards check: not applicable — scope-none. Diff touches only ac-daemon audio backends/handler validation, ac-cli's stop renderer, and ZMQ.md; none is a Tier-1 module in the document map (thd.rs, filterbank.rs, weighting.rs, noise.rs, reference_levels.rs, ccir468.rs, loudness/, sweep/mod.rs citations) and no measurement value, unit, or display format changes — plot_ir's analysis output is explicitly out of scope per triage and I confirmed the sweep/report generation code path is untouched by the diff. scope-none stands; no basis to raise it to tier-1.

correctness issues

none found. Independently re-verified (not carried over from a prior pass):

• checked_log_freq_point_count vs log_freq_points: read both, formulas match exactly.
• The existing non-cancellable play_and_capture used by τ calibration (handlers/calibrate/tau/measure.rs:185) is byte-for-byte unchanged (git diff on that file is empty) — the design's stated preservation of calibration behavior holds.
• JACK cancel-path ordering (silence then one_shot_active=false) is identical to the pre-existing normal/timeout exit ordering in the same function — not a new race.
• ac-view has no reference to "stopped"/"stimulus" (confirmed via grep across the crate) — matches the architect's "no stop-reply change" call, no missed wire consumer.
• ac-cli's render_success matches the UX spec's exact output ( stopped plot_ir / stimulus silent, empty-stopped-list renders no invented command name) — both cases covered by tests, both ran green.

test coverage gaps

coverage is adequate for everything checkable from the tree. The one remaining gap is the rig-measured evidence for the assumed ceilings (memory/CPU/stop-latency at each boundary) — that's requires-rig, not a missing-test finding, since no in-tree test can measure real hardware resource consumption.

scope issues

none. All touched files are within the architect's file manifest on #427.

Build/test independently re-run at tip f9c62a0d: cargo test --workspace (all crates, 0 failed, exit 0), cargo clippy --workspace --all-targets -- -D warnings (clean), cargo fmt --check (clean), cargo check -p ac-daemon --features cpal-audio (clean).

verdict

approve
Implementation correctly enforces every architect-approved (assumed) ceiling from one policy location, cancellation is correctly threaded through all three backends with no new race, wire/CLI rendering matches the UX decision exactly, and the full workspace gate is green — the only open item is a physical-rig measurement of whether the assumed values (not their enforcement) hold up, which is requires-rig's exact purpose, not grounds to withhold approval. Correction from the prior review pass on this same tip: that pass found no correctness, test, or scope defect anywhere in the PR and still returned request-changes, reasoning that a flagged assumed criterion "withholds in-review until the gap closes." That skips the routing table this role's own spec gives for exactly this situation: an unresolved assumed criterion whose implementation correctly enforces the specified value, and whose remaining gap only a physical rig can close, routes to approve + requires-rig — explicitly not request-changes, and explicitly not something a developer revision can address (confirmed independently: there is no in-tree edit, derivation, or test that produces real peak-memory/CPU/stop-latency numbers). The four developer-agent responses on this PR made this same point each pass; they were correct, and the label state is being corrected accordingly.

rig verification required

Run plot_ir at each documented ceiling — 60.0s duration, 60.0s tail, 32 harmonics, a 1,048,576-sample window, and a request whose sweep expands to 10,000 points — and record peak memory, peak CPU, and stop-to-silence latency for each. Falsifying value: any of these exceeding what a legitimate rig workflow can tolerate (stop-to-silence latency that isn't "prompt" by #427's own problem statement, or memory/CPU that starves a concurrent rig session) means the corresponding constant in handlers/mod.rs (MAX_STIMULUS_DURATION_S, MAX_IR_HARMONICS, MAX_IR_WINDOW_SAMPLES, MAX_SWEEP_POINTS) needs revision. No existing block in rig/rig-verify-queue.md covers this — needs a new one. requires-rig stays on this PR; only a human clears it, and only after that measurement exists.

[mkovero]

codex qa — PR #437 at f9c62a0

verdict: fail

findings

[severity: major] [confidence: high]

• location: ac-rs/crates/ac-daemon/src/handlers/admin.rs:42-72
• problem: Named stop replies unconditionally attest stimulus: "silent" even when another allowed output worker remains active.
• mechanism: The named branch removes and joins only the requested worker, while the busy policy permits Output, Input, and Transfer groups to coexist. The reply then emits stimulus: "silent" without checking workers left in the map or their groups. The same false attestation occurs when the requested name does not exist.
• failure scenario: Start generate (Output) and monitor_spectrum (Input), then send {"cmd":"stop","name":"monitor_spectrum"}. The monitor is joined, generate continues driving audio, but the reply says "stimulus":"silent". A nonexistent target while generate runs produces the same contradiction.
• evidence: Read handlers/admin.rs:42-72, handlers/mod.rs:60-86, workers.rs:174-188, and the named-stop wire contract in ZMQ.md:735-757. The group table and check_busy establish that different non-Exclusive groups can coexist; the handler establishes that only the named handle is removed before the unconditional attestation.
• disproof attempted: Checked whether plot-family exclusivity or the CLI’s unnamed stop made the state unreachable. plot_ir itself is Exclusive and the CLI stops all workers, but the changed ZMQ reply applies to the existing public named-stop form for every worker; the concurrency guard explicitly allows Output plus Input/Transfer, so the false state remains reachable by wire clients.
• recommendation: Only emit stimulus: "silent" after establishing that no output-driving worker remains (including the no-match case), or narrow/redesign the field so it attests only to the targeted stimulus worker; add a protocol regression covering named stop while an Output worker remains active.

gate

Claude QA workspace gate: inherited at current tip f9c62a0d
Codex targeted tests: not needed; the contradictory branch is established directly by the handler and concurrency-group policy

unaddressed open questions

None.

The architect-approved numeric ceilings remain assumed; the separating measurement is the existing requires-rig run of peak memory, CPU, and stop-to-silence latency at each ceiling. This does not alter the code verdict above.

scope

none

[mkovero]

Revision completed at 28a051af.

Design basis: before revising I checked linked issue #427 for architect or UX comments newer than the branch's original f9c62a0d commit. There are none. I revised against the architect's Option A decision in #427 (comment) and the UX decision in #427 (comment).

Point-by-point response to the needs-work finding:

1. Unconditional named-stop silence attestation: agreed. handlers/admin.rs now includes stimulus: "silent" only when the worker map is empty after selecting workers to stop and after the selected handles have joined. A named stop that leaves any worker running omits the field, so it cannot make a global silence claim while generate, transfer_stream, or another potentially output-driving worker remains. Stop-all and exclusive plot_ir cancellation still return the required confirmed-silent field.
2. No-match case: addressed by the same condition. A nonexistent named target no longer claims silence if another worker remains; it may confirm silence only when the worker map is already empty.
3. Protocol regression: added named_stop_does_not_claim_silence_while_output_remains. It starts concurrent generate (Output) and monitor_spectrum (Input), named-stops the monitor, asserts the reply omits stimulus, then named-stops the generator and asserts stimulus: "silent" appears.
4. Wire contract: updated ZMQ.md to state that the silence field is conditional and omitted from named-stop replies while another worker remains.

Verification at the revised tree:

• cargo test -p ac-daemon --test it_protocol named_stop_does_not_claim_silence_while_output_remains — 1 passed
• cargo test --workspace — passed (including 107/107 daemon protocol tests; configured hardware-only tests remain ignored)
• cargo clippy --workspace --all-targets -- -D warnings — passed
• cargo fmt --check — passed
• cargo check -p ac-daemon --features cpal-audio — passed

No review point is left disputed. Per the invocation instruction, I left all PR labels unchanged.

[mkovero]

Codex QA did not proceed: the current tip 28a051af postdates the latest actual Claude QA review, which covers f9c62a0d. A fresh Claude QA pass at 28a051af is required before independent Codex QA can review this tree. No labels were changed.

[mkovero]

Re-review. Commit range reviewed: f9c62a0dc27633d0e0d6a29a5bff6cca7b995ab7..28a051af7fc902a76a1aca73d37cd693b4b42b83 (one commit, "fix: make stop silence attestation truthful"). Prior QA comment re-read: the superseding review at unchanged tip f9c62a0d (verdict approve + requires-rig), which is the one this pass checks against — it supersedes the earlier request-changes pass on the same tip.

mechanical gate (re-run at tip 28a051af, full workspace, not per-crate)

• cargo test --workspace: 0 failed across every crate (ac-core, ac-daemon incl. 108 it_protocol tests now including the new one, ac-cli, ac-scene, ac-view). Full pass, not filtered.
• cargo clippy --workspace --all-targets -- -D warnings: clean.
• cargo fmt --check: clean.
• cargo check -p ac-daemon --features cpal-audio: clean.

delta review (f9c62a0d..28a051af)

Three files: ac-daemon/src/handlers/admin.rs, ac-daemon/tests/it_protocol/out_of_range.rs, ZMQ.md. This is the developer's response to the intervening agent: codex-qa finding (major, at f9c62a0d): named stop unconditionally attested stimulus: "silent" even when a non-conflicting worker (different concurrency group) remained running.

Fix read in full (admin.rs:42-78): the stimulus field is now set only when workers.is_empty() evaluated inside the same lock guard that performed the removal/drain, before the guard drops. Traced all three reachable paths:

• stop-all (target is None): drains the whole map unconditionally → is_empty() always true after → unconditional silent, same as before. No regression to the common CLI path (ac-cli stop.rs only ever sends unnamed stop).
• named stop, target found, was the only worker: map empty after removal → silent attested, correct.
• named stop, target found, another worker (different group) remains: map non-empty → stimulus omitted. This is the fixed case.
• named stop, target not found: nothing removed; is_empty() reflects the real pre-existing state, so a bogus name while another worker is running correctly omits the field too (codex-qa's secondary "no-match" case).

Confirmed the concurrency claim behind the fix directly rather than trusting the codex finding's assertion: read handlers/mod.rs::check_busy (workers.rs:182-186 for the group table) — generate is Group::Output, monitor_spectrum is Group::Input, and check_busy only blocks same-group or Exclusive collisions, so Output+Input concurrently running is real reachable state, not a hypothetical.

No lock-scope regression: no_workers_remain is computed and captured before the block ends and the lock drops, same critical section as the removal/drain — no TOCTOU window opened between the emptiness check and the join.

New test (named_stop_does_not_claim_silence_while_output_remains, out_of_range.rs): starts generate (Output) + monitor_spectrum (Input), named-stops the monitor, asserts stimulus is absent (.get("stimulus").is_none(), not just falsy), then stops generate and asserts stimulus: "silent" reappears. Reachable against the named defect: pre-fix code set the field unconditionally, so the first assertion would fail against f9c62a0d's admin.rs. Ran it in isolation to confirm: passes at 28a051af.

ZMQ.md: updated prose correctly describes the new conditional (lines 754-759). Minor: the example reply block at line 751 still shows {"stimulus": "silent"} unconditionally with no comment marking it optional — the prose immediately below is unambiguous about the condition, so this doesn't rise to a correctness or scope issue, just a note.

Consumer check: ac-cli/src/commands/stop.rs::render_success (unchanged by this delta) already reads the field with .get("stimulus").and_then(|v| v.as_str()) — Option-shaped, already tolerant of absence. ac-view has no reference to "stimulus" on the stop path (grepped ac-view/src, ac-cli/src — only hit is the same stop.rs). No consumer breaks from the field becoming conditional.

points from my prior (superseding) review — status

1. Cancellation terminates stimulus/capture on all three backends — untouched by this delta, still holds.
2. Numeric budget ceilings tagged assumed, enforcement correct, only rig evidence can validate the values — untouched by this delta. Checked Make plot_ir cancellable and bound all protocol-controlled work budgets #427 for new human comment: none since the architect/UX comments already reviewed (12:41–12:58 UTC 2026-09-01); requires-rig gap is still open, unchanged by this push.
3. Rejected requests emit no audio, observable error — untouched.
4. Regression coverage (cancellation, boundaries, non-finite, overflow) — untouched, and now extended by the new named-stop test.
5. "correctness issues: none found" — this was the miss: my prior pass didn't check the named-stop/other-worker-active interaction that codex-qa caught. That gap is now closed by this delta; verified the fix directly rather than taking codex-qa's or the developer's word for it (see above).
6. scope: admin.rs was already in the architect's file manifest and in the PR body's own file list from the first pass — this delta doesn't touch anything new outside it.

standards conformance

standards check: not applicable — scope-none (unchanged from prior pass). Delta touches ac-daemon handler logic (a boolean attestation condition, not a measurement value, unit, or display format), a protocol test, and wire documentation prose — none is in docs/architecture/standards.md's Tier-1 document map (re-checked: thd.rs, filterbank.rs, weighting.rs, noise.rs, reference_levels.rs, ccir468.rs, loudness/, sweep/mod.rs citations — admin.rs is in none of these).

correctness issues

none found in the delta.

test coverage gaps

none in the delta. The new test is reachable, asserts absence with .is_none() (not just a falsy check), and exercises real concurrency-group state rather than a fake.

scope issues

none.

verdict

approve
Delta correctly fixes the codex-qa-identified false silence attestation, the fix is verified independently (lock scope, concurrency-group reachability, all three reply paths traced by hand), the new regression test is reachable against the named defect, and the full workspace gate is green at tip 28a051af. All open points from the prior review (requires-rig's numeric-ceiling gap) are unchanged by this push and remain open on the same terms as before.

rig verification required

Unchanged from the prior review: run plot_ir at each documented ceiling (60.0s duration, 60.0s tail, 32 harmonics, a 1,048,576-sample window, a 10,000-point sweep) and record peak memory, peak CPU, and stop-to-silence latency. Falsifying value: any exceeding what a legitimate rig workflow can tolerate. No block in rig/rig-verify-queue.md covers this yet — needs a new one. requires-rig stays; only a human clears it, and only after that measurement exists. This delta does not touch or resolve this gap.

label note

claude-approved was applied to this PR before this push (at tip f9c62a0d) and was not removed when 28a051af landed — per the approval-voiding rule, an approval covers a specific commit and a later push (including one addressing a codex-qa finding) reverts that. Removing it now as part of this re-review since the pusher didn't; re-applying it below reflects this pass's fresh verdict at the new tip, not a carry-over of the stale label.

[mkovero]

codex qa — PR #437 at 28a051a

verdict: pass

gate

Claude QA workspace gate: inherited at current tip 28a051af
Codex targeted tests: cargo test -p ac-daemon --test it_protocol plot_ir_stop_cancels -- --nocapture (2 passed); cargo test -p ac-daemon --test it_protocol plot_family_rejects_resource_budgets_before_spawn -- --nocapture (1 passed); cargo test -p ac-daemon --test it_protocol named_stop_does_not_claim_silence_while_output_remains -- --nocapture (1 passed)

unaddressed open questions

None.

The architect-tagged numeric ceilings remain assumed. The separating rig measurement is peak memory, peak CPU, and stop-to-silence latency at the 60 s duration/tail, 32-harmonic, 1,048,576-sample-window, and 10,000-point ceilings; the existing requires-rig label correctly preserves that release gate.

scope

none

[mkovero]

rig-2026-09-14-pr437-plot-budget — rig record

Date (UTC): 2026-09-14 · Rig: pupu · Operator: Markus Kovero · Run by: Claude Code session
Verifies PR #437 (closes #427), QA's "rig verification required" field: plot_ir at each documented
ceiling (60 s duration, 60 s tail, 32 harmonics, 1,048,576 window) and a 10,000-point sweep; peak
memory, peak CPU, stop-to-silence latency.

Build under test

rev 28a051a (dirty 0), rustflags -C target-cpu=x86-64 -C link-arg=-fuse-ld=mold,
staged /home/mui/ac-test/28a051af7fc9-x86_64, sha256 verified on rig; not installed.
f51107b8…a3b15 ac · bf9a5639…96f8 ac-daemon · e3f0bdb4…2d21f ir_probe · 9db6187e…ab12c transfer_probe · 9725d67c…85aa3 it_loopback_ir
Scripts: PR #441 branch rig-testing-runbook @ 4ff325a. Built on dev host, not on rig.

Pre-flight (19:44:10Z) — all PASS

JACK active 96000/256/-S; port order analog block capture 1-8 (14 captures); ALSA baseline 93=2 94=2 89=2 81=20,0
82=0,0 90=on,off 102=on; rig ac config drive_max_dbfs -40, out 0 in 0 ref-out 1 ref-in 1; no daemon; xruns last 10 min 0.

Physically connected (probed 19:49:24Z, probe-outputs.sh --level -60 --outputs 1)

ac output 1 = system:playback_2 → IN2 tone −57.5 dBFS (+2.5 dB loopback gain); IN1 mic tone −145.6 dBFS
(speaker not driven); IN3–8 floor. Output 0 (speaker) not probed, not driven this session.

Clock state

FF400 internal clock, snd_fireface, jackd -S 96 kHz period 256; unchanged.

Emission consent

• Operator, in session ~19:39Z: "you may proceed with emissions too after 10min". First emission 19:49:24Z.
• Scope: loopback AN2→IN2 only; ESS, stepped tones/sines; ≤ −40 dBFS nominal; every run bounded or stopped.
• Ceiling: standing −40 dBFS, server-side drive_max_dbfs −40.0 in the running daemon's config (isolated
  HOME /home/mui/ac-test/runs/437/home, output_port system:playback_2 / input_port system:capture_2 pinned).
  Observed ADC peaks −37.1 dBFS = −40 nominal +2.5 dB gain (+crest), consistent.

Method

pyzmq over SSH tunnel to ac-daemon --local :15556/15557. Rig-side per run: pidstat -h -r -u -p 1;
jack_rec -b 24 system:capture_2. Stop sent by staged ac stop on the rig between two rig date stamps (same
clock as capture). Silence = last 5 ms RMS block above pre-stimulus floor +20 dB. Daemon identity (status pid →
/proc exe, config readback) checked every run.

Silent pre-checks — one past each ceiling refused with "stimulus silent": duration 60.001, tail_s 60.001,
n_harmonics 33, window_len 1048577, plot_level steps 10001, plot 20–20000 Hz ppd 3334 (10002 points).

Runs (all −40 dBFS nominal, loopback)

run	request	outcome	peak RSS	CPU peak/mean	stop→silence
1	plot_ir dur 60, tail 0.5, full	error "capture timeout after 60.5s" @+63.1 s; stimulus played to +60.46 s	185.2 MiB	52/4.4 %	n/a
2	same, ac stop @+20 s	INVALID: staged ac refused "daemon at localhost:15556 belongs to a different HOME"; tunnel stop @~+48 s reply 28 ms {stimulus silent, stopped plot_ir}	—	—	—
2b	same, ac stop (HOME matched) @+20.07 s	reply 27.3 ms "stopped plot_ir / stimulus silent"	181.8 MiB	46/4.8 %	last signal +12.0 ms after reply; after reply+20 ms max −90.6 dBFS (floor −104.4)
3	plot_ir dur 2, tail 60, full	error "capture timeout after 62.0s" @+64.1 s	123.1 MiB	7/3.5 %	n/a
4	plot_ir dur 20, tail 11, harm 32, win 1048576 (largest fitting capture)	IR +33.8 s, report +34.7 s, done +35.2 s	414.1 MiB	116/9.3 %	n/a
5	plot_level −60→−40, 10000 steps, 0.1 s, stop @+90 s	434 pts; reply 79.9 ms stimulus silent; done xruns 0	170.3 MiB	9/5.9 %	silent 39.6 ms before reply; after reply −103.5 dBFS
6	plot 20–6324.555 Hz ppd 4000 (10000 pts), 0.1 s, stop @+90 s	433 pts; reply 161.1 ms stimulus silent; done xruns 0	170.3 MiB	10/5.8 %	silent 24.3 ms before reply; after reply −101.8 dBFS

Pass stated before running: completes or stops with silence attested and IN2 at floor within one round trip of
the reply; RSS/CPU leave room for a concurrent session on 4 cores / 7.8 GiB.

Finding 1 — FAIL: MAX_STIMULUS_DURATION_S = 60 admits plot_ir requests the JACK backend cannot complete

Runs 1 and 3 emitted, then timed out with no IR. jack_backend.rs:28 RING_CAPACITY = 16*192_000 = 3,072,000
samples; play_and_capture waits for rings.occupied() >= n_total (:412) with deadline n_total/sr + 2 s (:405).
At 96 kHz the ring holds 32.0 s (48 kHz: 64 s; 192 kHz: 16 s), so duration + tail_s > 32 s can never complete.
duration and tail_s are budgeted separately (each ≤ 60 s) → admitted capture up to 120 s, 3.75× the 96 kHz ring.
This is QA's falsifying case: a request inside every ceiling emits up to 60 s and produces nothing. Fix direction:
bound (duration + tail_s) × sample_rate against the ring and refuse silently, or size the ring from the budget.

Finding 2 — pass: stop-to-silence is prompt

plot_ir mid-sweep: reply 27 ms, last above-floor block 12 ms after reply (within JACK/converter round trip).
plot_level/plot: IN2 silent 24–40 ms before the 80/161 ms reply. Attestation matched capture on every valid stop.

Finding 3 — readout: resources

Largest completing plot_ir 414 MiB RSS, 116 % CPU burst (~4 s deconvolution), mean 9 %. All else ≤ 185 MiB,
mean ≤ 6 %. None starves this rig. Combined literal ceilings (60+60 s, 32, 1048576) not run: finding 1 makes it
time out before analysis; its analysis memory is owed once the ring/budget changes. 10,000-point plot_level/plot
at 0.1 s: ~0.207 s/point → ~35 min emission; RSS flat at 434 points, not measured at 10,000.

Confound

• jack_rec start offset vs its date stamp unmeasured; a late start makes stop→silence read short by up to tens of
  ms. Finding 2 margins are that scale — read as "prompt, one-round-trip order", not exact latency.
• Stop during plot_ir tail not measured. Memory at 10,000 completed points not measured. 96 kHz only.

Rig state left behind (after #437 runs)

Daemon stopped; no jack_rec/pidstat; jackd xrun lines last 25 min: 0. ~/.config/ac/config.json untouched
(isolated HOME; probe-outputs restored channels). ALSA, JACK, mic, phantom, gain unchanged.
Artefacts: rig /home/mui/ac-test/runs/437/, /home/mui/ac-test/runs/20260914T194924Z-probe-outputs.

What should happen next

1. PR fix: cancel and bound plot_ir work #437: finding 1 blocks — bound total capture against the ring at the running rate (or size the ring),
   then rerun runs 1, 3 and the combined literal-ceiling request on the new tip.
2. requires-rig stays on fix: cancel and bound plot_ir work #437 until that rerun exists (human clears).
3. Low: rig scripts that stop a manually started daemon must pass its HOME to ac.

[mkovero]

Re-review. Commit range reviewed: ec63245f62019d4062f90191750f23ed5d21436c..cf491623ca719ba05377a9d6da21935fcd8ba642 (one commit: cf491623 "fix: bound plot's derived per-point capture duration before spawn"). Prior review was posted at ec63245f.

mechanical gate (re-run at tip cf491623, full workspace, isolated CARGO_TARGET_DIR=target-qa-437)

• cargo fmt --check: clean.
• cargo clippy --workspace --all-targets -- -D warnings: clean, zero warnings.
• cargo test --workspace: clean. Every test result: line in the run is 0 failed (checked by grepping the full log for FAILED|error\[|panicked — no hits). The it_protocol suite (plot_family_rejects_resource_budgets_before_spawn, which carries the delta's two new assertions) passed. Note: the two pre-existing it_set_drive timing flakes recorded in my ec63245f pass did not reproduce this run — consistent with them being load-dependent flakes, not a regression either way.

delta review (ec63245f..cf491623)

Two files: handlers/audio/plot.rs (+21) and tests/it_protocol/out_of_range.rs (+18), both inside plot's existing scope.

The defect. plot's worker floors each point's capture at dur = f64::max(duration, 3.0 / freq) (plot.rs:222) and hands dur to capture_block. On the JACK backend that reaches Duration::from_secs_f64(duration_s + 2.0) (jack_backend.rs:421) — Duration::from_secs_f64 panics on a value outside its representable range. Before this delta, start_hz had no ceiling relating it to the duration budget: a request like start_hz: 1e-300 makes 3.0 / start_hz ≈ 3e300, which reaches that panic. I traced this chain myself (plot.rs → capture_block → jack_backend.rs:446 → rings.capture_block → the Duration::from_secs_f64 call at jack_backend.rs:421) rather than taking the commit message's claim on faith.

The fix. Before port resolution or worker spawn, plot.rs:146 now computes max_point_duration = f64::max(duration, 3.0 / start_hz) and rejects if it's non-finite or exceeds MAX_STIMULUS_DURATION_S. This is the same dur formula the worker uses, evaluated at start_hz specifically. I checked that's sound, not just plausible: checked_log_freq_point_count (called earlier in the same function, plot.rs:131) already refuses stop < start and requires start > 0, and log_freq_points (handlers/mod.rs:509) builds the grid as start * (stop/start)^(i/(n-1)), monotonically increasing in i — so start_hz is always the smallest frequency in the sweep and therefore always the largest 3.0/freq floor. The guard covers the worst case exactly, not an approximation of it.

Test reachability, checked against the actual defect, not the commit message. assert_budget_rejection (out_of_range.rs:181) asserts ok == false and status.busy == false after the call. In the pre-fix code, plot returns {"ok": true, ...} synchronously right after spawning the worker — the panic happens later, in the spawned thread. So this test would have failed on the very first assertion (ok == false) against the unfixed code: it's a genuine regression test for the defect it names, not a test of the fixed code's own output. Ran cargo test -p ac-daemon --test it_protocol out_of_range::plot_family_rejects_resource_budgets_before_spawn standalone — 1 passed.

points from my ec63245f review — status

1. Cancellation terminates stimulus/capture on all three backends — untouched by this delta, still holds.
2. Numeric budget ceilings tagged assumed — untouched, still open. requires-rig stays.
3. Rejected requests emit no audio, observable error — extended by this delta to a new rejection path (start_hz-derived duration), same pattern, still holds.
4. Regression coverage — extended: closes a real gap codex-qa found post-approval (unbounded per-point floor bypassing the duration ceiling), not previously covered by any test in this PR.
5. Coupled-constant / misleading-comment findings from the 83b8d01a pass — unaffected by this delta.
6. Scope — only plot.rs and out_of_range.rs touched, both already in the architect's #427 file manifest.

spec coverage

criterion	provenance	covered	notes
Stopping plot_ir terminates stimulus and capture promptly on every supported backend	measured	✓	Unchanged by this delta.
Duration, tail, point density, step count, harmonic count, and window length are rejected before worker spawn when outside explicit finite budgets	assumed (architect, #427)	✓ (mechanism), ceiling values still open	This delta closes a real hole in this criterion — start_hz could bypass the duration ceiling via the per-point floor — but the numeric ceiling values remain rig-unverified, unchanged from prior passes.
Rejected requests emit no audio and return an observable error	measured	✓	Extended to the new rejection path; assert_budget_rejection checks both busy == false and error text.
Regression coverage includes cancellation during stimulus, cancellation during tail, each budget boundary, non-finite input, and integer-conversion overflow	measured	✓	Strengthened — the new tests cover a boundary this criterion implies (a "budget boundary" on the derived per-point duration) that was previously unexercised.

standards conformance

standards check: not applicable — scope-none (issue #427's own label, unchanged from every prior pass on this PR). This delta is a resource-budget validation guard and its test; it does not touch a measurement formula, unit, or displayed value.

correctness issues

1. ac-rs/ZMQ.md:1215-1234 (plot section) — not touched by this delta. It documents duration (≤60s) and the derived point-count ceiling (≤10000), but not the new implicit rejection: a start_hz low enough that 3.0/start_hz alone exceeds MAX_STIMULUS_DURATION_S is now rejected, even when duration itself is in range. ZMQ.md is called out in this repo as authoritative for both the Rust and Python clients, and this PR's own description states it "document[s] the protocol limits." Non-blocking (the code and test are correct; a client constructing a start_hz in the sub-Hz range is not a realistic path — CLI default is 20 Hz), but worth a one-line addition to plot's request docs so a wire client isn't surprised by an undocumented rejection reason.

test coverage gaps

none in the delta — both new cases (the boundary at start_hz: 0.001 and the exact codex-qa repro at start_hz: 1e-300) are reachable and exercise the actual panic path, verified above rather than assumed from the commit message.

scope issues

none.

verdict

approve
The delta is a correct, narrowly-scoped fix for a real daemon-panic path (traced independently through capture_block → Duration::from_secs_f64, not just trusted from the commit message), its guard formula is provably the worst case over the sweep grid, and its regression tests are reachable — verified to fail against the pre-fix behavior, not just pass against the post-fix code. Full workspace gate (fmt, clippy -D warnings, test --workspace) is clean at tip cf491623. All open items from prior passes (assumed numeric ceilings, requires-rig) are unchanged by this push; one non-blocking doc gap noted above (ZMQ.md not updated for the new rejection reason).

sent back to

no

rig verification required

Unchanged in substance from all prior passes: run plot_ir at the documented ceilings (60.0s duration, 60.0s tail, 32 harmonics, a 1,048,576-sample window, a 10,000-point sweep) and record peak memory, peak CPU, and stop-to-silence latency. This delta doesn't touch or resolve that gap. requires-rig stays; only a human clears it.

label note

claude-approved was absent going into this push (correctly voided when cf491623 landed after the ec63245f approval — in-review and requires-rig were left in place by the pusher, consistent with the approval-voiding convention). Re-applying claude-approved now for this pass's fresh approve verdict at cf491623.

[mkovero]

codex qa — PR #437 at cf49162

verdict: fail

findings

[severity: major] [confidence: high]

• location: ac-rs/crates/ac-daemon/tests/it_protocol/plot_ir.rs:82-118
• problem: The stimulus/tail cancellation regressions can pass without proving that plot_ir observed cancellation, so they do not reliably cover the original non-cancellable-handler defect.
• mechanism: The tests only assert a prompt stop reply, its fields, and the post-stop busy state. Completed worker handles remain in the workers map until stop removes them, so those assertions also succeed after natural worker completion. Meanwhile FakeEngine::play_and_capture synthesizes the entire capture immediately; only the new play_and_capture_cancellable override adds real-time pacing. Reverting plot_ir to its original play_and_capture call therefore bypasses the five-second pacing that makes the elapsed-time assertion meaningful, particularly in the 0.1 s stimulus / 5 s tail case. No assertion observes that the stop flag caused the capture path to return early.
• failure scenario: A regression changes plot.rs:859 back to eng.play_and_capture(&scaled, tail_s). The fake produces the requested tail immediately, the worker can finish normally, and stop still removes its retained handle and reports stopped: ["plot_ir"], stimulus: "silent", and busy: false; the named cancellation regression can remain green even though JACK/CPAL would again block through the requested tail.
• evidence: Read plot_ir.rs:82-118, audio/fake/mod.rs:248-280, workers.rs:139-160, handlers/mod.rs:164-176, and the handler call at plot.rs:859. Ran cargo test -p ac-daemon --test it_protocol plot_ir_stop_cancels -- --nocapture: both current tests pass in 0.46 s, confirming the present path but providing no separate cancellation observation.
• disproof attempted: Searched for automatic removal of naturally completed workers, a fake-backend cancellation hook/counter, or another test that asserts cancellation rather than completion. spawn_worker only returns a handle, handlers insert it, and removal occurs through stop; no cancellation observation or second regression exists.
• recommendation: Make the integration test distinguish cancellation from normal completion—for example, subscribe before launch and assert the cancelled run emits neither the impulse-response/report nor normal done frame, or add an equally direct test-only cancellation acknowledgement—so reverting the handler to the non-cancellable method fails deterministically.

[severity: minor] [confidence: high]

• location: ac-rs/ZMQ.md:1229-1232
• problem: The documented plot limits omit the new rejection when max(duration, 3/start_hz) exceeds 60 seconds.
• mechanism: plot.rs:145-155 now imposes an effective lower bound on start_hz through the per-point capture floor, but the wire documentation lists only the explicit duration and point-count constraints.
• failure scenario: A wire client sends start_hz: 0.001, stop_hz: 0.001, and an otherwise valid duration based on the documented ranges; the daemon rejects it for an undocumented protocol limit.
• evidence: Compared the complete diff and current ZMQ.md plot request text with handlers/audio/plot.rs:137-155 and the two new direct-protocol rejection cases.
• disproof attempted: Searched the plot-family request documentation for 3/start_hz, per-point duration, or a corresponding start-frequency bound; none is present.
• recommendation: Document that the derived per-point duration max(duration, 3/start_hz) must also be at most 60 seconds.

gate

Claude QA workspace gate: inherited at current tip cf491623
Codex targeted tests: cargo test -p ac-daemon --test it_protocol plot_ir_stop_cancels -- --nocapture — 2 passed, 0 failed

unaddressed open questions

The architect-assumed numeric ceilings still need the recorded rig measurements of peak memory, CPU, and stop-to-silence latency; the existing requires-rig label correctly retains that human-only gate.

scope

none

[mkovero]

Revision completed at 942c0e27, addressing the codex-qa fail at cf491623 (2026-09-15T09:50:17Z).

design basis

Checked issue #427 for an architect/UX comment newer than this branch's base commit (cf491623, 2026-09-15T08:58:05Z). None exists — the newest comments on #427 are the architect (12:41Z) and UX (12:48Z) comments from 2026-09-01, both well before this branch's every commit. No design change under this revision.

the findings

1. [major] Cancellation tests don't prove cancellation, only a prompt reply + busy:false.

Agreed and reproduced before fixing: with plot.rs's handler temporarily reverted to the non-cancellable eng.play_and_capture(&scaled, tail_s), both plot_ir_stop_cancels_during_stimulus and plot_ir_stop_cancels_during_tail still passed under the old assertions — the fake backend's non-cancellable path has no pacing sleep at all, so the worker completes and publishes within milliseconds, well before the test's settle sleep even starts, and stop on an already-finished (and, it turns out, already self-removed) worker still returns ok:true/stimulus:"silent"/busy:false.

Fix: after stop, drain every already-queued PUB frame and assert none of them is this request's measurement/impulse_response, measurement/report, or done frame (all three carry "cmd":"plot_ir"). Verified this actually catches the regression, not just plausibly should: with the revert restored, plot_ir_stop_cancels_during_stimulus now fails on the new impulse-response assertion, and plot_ir_stop_cancels_during_tail fails even earlier (stopped["stopped"] == []) because the worker had already vacated the workers map by the time stop was sent — which is itself evidence for the review's premise (a finished worker's completion is invisible to the old assertions) even though the map-removal timing differs slightly from the review's stated mechanism. Reverted the temporary change back out before committing; git diff on plot.rs is empty.

2. [minor] ZMQ.md doesn't document the new max(duration, 3/start_hz) rejection for plot.

Agreed, fixed. Added a paragraph to plot's request docs in ZMQ.md stating the per-point floor, that start_hz carries the largest one on the non-decreasing grid, and that the daemon rejects pre-spawn when that floor alone is non-finite or exceeds the 60 s ceiling — even when duration itself is in range.

verify gate at 942c0e27

• cargo fmt --check (workspace): clean
• cargo clippy --workspace --all-targets -- -D warnings: clean
• cargo test -p ac-daemon --test it_protocol plot_ir: 8 passed, 0 failed
• cargo test --workspace (isolated CARGO_TARGET_DIR): every crate green except one pre-existing, unrelated flake — it_set_drive.rs::drive_on_raises_the_captured_level_and_off_returns_it_within_one_frame (timing assertion on a fixed frame window; same class the ec63245f QA re-review already recorded as load-dependent and out of this PR's file scope). Reran it alone, single-threaded: passed. it_set_drive.rs is untouched by this delta.
• cargo check -p ac-daemon --features cpal-audio: clean

scope

Only ZMQ.md and crates/ac-daemon/tests/it_protocol/plot_ir.rs touched — both already in this PR's file list and the architect's #427 manifest. No source (non-test) behavior change; no wire-schema change beyond the doc addition (which documents existing cf491623 behavior, not new behavior).

points I don't dispute

None — both findings are correct as filed and addressed above.

label note

Per this invocation's instruction I left every label untouched. Flagging rather than silently leaving it: claude-approved on the PR right now attests to cf491623, which this push supersedes — under this PR's own approval-voiding convention (see the 83b8d01a/28a051af/a5b7d761 QA comments above), that approval is stale as of this commit. I have not removed it per the explicit "do not change labels" instruction for this run; a human or the next QA pass should treat it as void pending a fresh review at 942c0e27.

[mkovero]

Codex QA did not review the current tree: the claude-approved approval predates commit 942c0e276d857c1c218089e0d162b27352a61f75 (committed 2026-09-15 10:02:33 UTC). A fresh Claude QA pass at the current tip is required before independent Codex QA can proceed. No labels were changed.

[mkovero]

Re-review. Commit range: cf491623ca719ba05377a9d6da21935fcd8ba642..942c0e276d857c1c218089e0d162b27352a61f75 (one commit, 942c0e27 "fix: prove plot_ir stop cancels rather than reaps a finished worker"). Prior review was posted at cf491623.

mechanical gate (re-run at tip 942c0e27, full workspace, isolated CARGO_TARGET_DIR=/home/mui/src/ac-wt/target/qa-437-tip942c)

• cargo fmt --check: clean.
• cargo clippy --workspace --all-targets -- -D warnings: clean, zero warnings.
• cargo check -p ac-daemon --features cpal-audio: clean.
• cargo test --workspace: clean. Grepped the full log (/home/mui/src/ac-wt/log/qa-437-tip942c-test.log) for FAILED|panicked|^error — no hits; every test result: line reads 0 failed. plot_ir::plot_ir_stop_cancels_during_stimulus and plot_ir::plot_ir_stop_cancels_during_tail both ok, both in the full run and standalone (cargo test -p ac-daemon --test it_protocol plot_ir::plot_ir_stop_cancels -- --test-threads=1, 2 passed). The loudness-under-load flake noted at the 83b8d01a merge pass did not reproduce this run.

delta review (cf491623..942c0e27)

Two files, both within existing PR scope: ac-rs/ZMQ.md (+7, docs) and ac-rs/crates/ac-daemon/tests/it_protocol/plot_ir.rs (+36, test only). No source file in ac-daemon/src touched.

What it does. Responds to a codex-qa finding that plot_ir_stop_cancels_during_{stimulus,tail}'s two assertions (prompt stop reply, busy:false) also pass when plot_ir simply finished on its own before stop was sent, because a finished worker's handle previously read as staying in the map until stop removed it. The fix adds a drain of every queued PUB frame after stop+status, asserting none of this request's measurement/impulse_response, measurement/report, or done frames appear.

Verified, not taken on the commit message's word:

• Every one of those three frames does carry "cmd":"plot_ir" in its payload — checked at plot.rs:954 (measurement/impulse_response), plot.rs:1110 (measurement/report), plot.rs:1136 (done) — so the test's payload["cmd"] != json!("plot_ir") filter can't accidentally skip a real completion frame or false-positive on another command's frame.
• No periodic background PUB traffic exists that could stall or fool the drain: every send_pub call site in ac-daemon/src (grep -rl send_pub crates/ac-daemon/src) sits inside a command handler, not a free-running thread — monitor_spectrum, the closest thing to a background loop, is itself spawned only by the monitor command (monitor/mod.rs:178), which this test never issues. So the drain loop's 50ms-timeout/1000-iteration cap is a safety bound, not something normally exercised.
• Reachability against the named defect: traced capture_block's non-cancellable sibling, play_and_capture (audio/fake/mod.rs:248) — it has no pacing sleep at all, unlike play_and_capture_cancellable's 10ms-chunk loop (audio/fake/mod.rs:262-281). A revert to the non-cancellable path finishes the worker thread almost immediately regardless of requested duration/tail_s.
• One correction to the test's own inline reasoning, found while tracing this: the comment states a finished worker's handle "stays in the workers map until stop removes it." That's not quite the mechanism — server.rs:300-306 reaps any worker whose thread is_finished() every iteration of the main loop, which polls at a fixed 10ms (server.rs:348), independent of whether stop is ever sent. So on the reverted code, whichever of the two scenarios' worker thread finishes fastest gets reaped before stop runs — failing the pre-existing stopped == ["plot_ir"] assertion — while a scenario whose thread is still mid-flight (heavier duration) stays in the map, gets joined synchronously inside stop(), runs to completion during that join, and only then fails on the new frame assertions. This is consistent with the commit message's claim that the two scenarios fail at different assertions, just via a different mechanism than the comment states. Non-blocking — the test is still reachable against the defect it names — but the comment misdescribes why, and a future reader relying on "stays until stop removes it" to reason about some other race would be reasoning from a false premise.

points from my cf491623 review — status

1. ZMQ.md:1233-1234 non-blocking gap ("a start_hz low enough to blow the per-point floor is now rejected, but ZMQ.md doesn't say so") — addressed. This delta adds exactly that text under ### plot (ZMQ.md:1236-1241), and I checked it against the code it describes: max(duration, 3.0/start_hz) and the 60s ceiling match plot.rs:145-153's max_point_duration guard and handlers/mod.rs:486's MAX_STIMULUS_DURATION_S = 60.0 exactly.
2. Numeric ceilings tagged assumed, rig evidence outstanding — untouched by this delta, still open.
3. requires-rig — untouched, stays. Only a human clears it.
4. Scope (only plot.rs/out_of_range.rs touched at cf491623) — this delta stays in scope: a doc file already listed in the PR's own file manifest, plus the existing plot_ir.rs test file. No new scope issue.
5. claude-approved label note (re-applied at cf491623) — voided again by this push, per the standing rule; codex-qa's comment on this PR already flagged that the label was stale relative to 942c0e27 and left it alone. This review re-validates the current tip; see label note below.

spec coverage

criterion	provenance	covered	notes
Stopping plot_ir terminates stimulus and capture promptly on every supported backend	measured	✓	Unchanged by this delta, but the delta strengthens how this is proven — see delta review above.
Duration, tail, point density, step count, harmonic count, and window length are rejected before worker spawn when outside explicit finite budgets	assumed (architect, #427)	✓ (mechanism), ceiling values still open	Unchanged from prior pass — this delta is docs/tests only, doesn't touch the ceiling values.
Rejected requests emit no audio and return an observable error	measured	✓	Unchanged by this delta.
Regression coverage includes cancellation during stimulus, cancellation during tail, each budget boundary, non-finite input, and integer-conversion overflow	measured	✓	Strengthened — cancellation coverage now proves the worker was actually interrupted (no completion frames), not just that stop replied promptly and status read idle.

standards conformance

standards check: not applicable — scope-none (issue #427's own label; reconfirmed on the issue directly this pass). Checked the delta against docs/architecture/standards.md's document map (/home/mui/src/ac/docs/architecture/standards.md:13-22): neither ac-daemon/src/handlers/audio/plot.rs nor ZMQ.md nor the it_protocol tests are listed modules, and this delta touches only a protocol-doc addition and a test — no measurement formula, unit, or displayed value changes.

correctness issues

none found. The one thing worth a maintainer's attention is non-blocking and noted above (test comment misdescribes the reap mechanism, though the test itself is reachable and correct).

test coverage gaps

none in the delta. The assumed-ceiling rig-evidence gap noted in every prior pass is unchanged — that remains a requires-rig matter, not a missing-test matter.

scope issues

none.

verdict

approve
Delta is docs + a strengthened regression test, both verified against the actual code paths rather than taken on the commit message. Full workspace gate (fmt, clippy -D warnings, test --workspace, check --features cpal-audio) is clean at tip 942c0e27. All open items from prior passes (assumed numeric ceilings, requires-rig) are unchanged by this push; the one previously-open non-blocking doc gap is now closed.

sent back to

no

rig verification required

Unchanged from all prior passes: run plot_ir at the documented ceilings (60.0s duration, 60.0s tail, 32 harmonics, a 1,048,576-sample window, a 10,000-point sweep) and record peak memory, peak CPU, and stop-to-silence latency. This delta doesn't touch or resolve that gap. requires-rig stays; only a human clears it.

label note

claude-approved predates 942c0e27 (codex-qa already flagged this on the PR and left labels alone). This pass re-validates the gate and the diff at the current tip with a fresh approve — claude-approved, in-review, and requires-rig all stay as currently set; no label change needed since they already match this review's outcome.

[mkovero]

codex qa — PR #437 at 942c0e2

verdict: fail

findings

[severity: major] [confidence: high]

• location: ac-rs/crates/ac-daemon/src/audio/jack_backend.rs:26-44,289-307,395-443,655-683
• problem: The fixed JACK capture ring only covers the accepted 120-second plot_ir maximum through 192 kHz, although the backend accepts the JACK server's actual sample rate without a 192 kHz ceiling.
• mechanism: start() copies Client::sample_rate() directly into the engine, while RING_CAPACITY is fixed at 120 * 192_000; play_and_capture_cancellable() waits for duration + tail_s samples to accumulate. The coupled-constant test enumerates only 44.1/48/96/192 kHz, so it stays green when the same relationship fails at a higher live JACK rate.
• failure scenario: Run JACK at 384 kHz and submit an otherwise-valid plot_ir request with duration: 60 and tail_s: 60. The handler accepts it, the backend requires 46,080,000 captured samples, but the ring holds 23,040,000 and drops newer samples once full. The request therefore plays the full stimulus and waits until its timeout instead of producing an IR, the same failure mechanism the 96 kHz rig run exposed before the ring enlargement.
• evidence: Read the live-rate assignment and fixed allocation in jack_backend.rs, the n_total wait condition in the same file, and SweepParams::n_samples() in ac-core/src/measurement/sweep/mod.rs. Searched the daemon/core tree for a sample-rate ceiling and found none. The repository explicitly exercises a 384 kHz device path in ac-core/src/visualize/mtw/ladder.rs:478-492. Targeted tests passed, including the ring test, confirming its current matrix does not cover this case. The rig record tested 96 kHz only and explicitly lists one rate as a confound.
• disproof attempted: Checked for a configuration, handler, engine-start, or analysis guard rejecting rates above 192 kHz; none exists. Checked whether the 384 kHz path is treated as unsupported; the repository instead requires it to gain an additional MTW stage. Checked whether cancellation avoids the failure; cancellation works, but an uncancelled valid request still cannot complete.
• recommendation: Couple capture capacity to the actual JACK sample rate and the accepted combined duration, or reject unsupported rates before any stimulus is emitted. Make the regression assert the relationship against the real maximum accepted engine rate (including 384 kHz if it remains accepted).

gate

Claude QA workspace gate: inherited at current tip 942c0e276d857c1c218089e0d162b27352a61f75
Codex targeted tests: cargo test -p ac-daemon request_budget_tests — 2 passed; cargo test -p ac-daemon ring_capacity_fits_stimulus_duration_and_tail_budget_at_every_supported_rate — 1 passed; cargo test -p ac-daemon --test it_protocol plot_ir_stop_cancels -- --test-threads=1 — 2 passed

unaddressed open questions

None from the QA or UX comments. requires-rig remains a separate human gate.

scope

none

[mkovero]

Response to codex-qa at 942c0e2: capture ring sized from the live sample rate

Change (audio/jack_backend.rs): the fixed RING_CAPACITY = 120 * 192_000 is replaced by meas_ring_capacity(sample_rate) = ceil(2 × MAX_STIMULUS_DURATION_S × sample_rate). MAX_STIMULUS_DURATION_S is imported from handlers, not duplicated. start() allocates the measurement ring from self.sample_rate, which is set from client.sample_rate() just before. At 384 kHz, a duration 60 + tail_s 60 request now gets a 46,080,000-sample ring instead of timing out against 23,040,000.

Memory: 120 s × 4 B/sample means 44 MiB per engine at 96 kHz (was 88 MiB, see the rig record's ring-wrap soak) and 176 MiB at 384 kHz. The rig record's resident-cost readout scales with that.

How this was committed: the developer session for this revise wrote the change, then stopped while waiting on a backgrounded cargo test --workspace. In headless mode nothing resumes it, so it never committed, pushed or commented, and master.sh misread that as "revise pushed nothing". The uncommitted change was committed as written, after cargo fmt --check, cargo clippy -- -D warnings and cargo test --workspace passed on it.

Known weakness, for review: meas_ring_capacity_fits_stimulus_duration_and_tail_budget_at_every_rate computes its expected sample count with the same formula meas_ring_capacity uses. cap >= n_total therefore holds almost by construction. It does not catch start() going back to a fixed capacity, as long as the function still exists. A test that asserts the ring actually allocated at start() for a given rate would.

[mkovero]

Re-review. Commit range: 942c0e276d857c1c218089e0d162b27352a61f75..c71b69956b09129293034e9db35778634a3ba2d5 (one commit, c71b6995 "fix: size the JACK measurement ring from the live sample rate"). Prior review was posted at 942c0e27; codex-qa filed a fail verdict against that tip after my approval (major finding: fixed RING_CAPACITY = 120 * 192_000 doesn't scale above 192 kHz, and the backend accepts JACK's actual live rate with no ceiling — a rig running at 384 kHz, the same path mtw::ladder already exercises, reproduces the exact "plays full stimulus, times out with no IR" failure the #437 rig finding originally caught). This commit is the developer's response to that finding.

mechanical gate (re-run at tip c71b6995, full workspace, isolated CARGO_TARGET_DIR=/home/mui/src/ac-wt/target/qa-437-tipc71b)

• cargo fmt --check: clean.
• cargo clippy --workspace --all-targets -- -D warnings: clean, zero warnings.
• cargo check -p ac-daemon --features cpal-audio: clean.
• cargo test --workspace: clean. Grepped the full log (/home/mui/src/ac-wt/log/qa-437-tipc71b-test.log) for FAILED|panicked|^error — no hits; every test result: line reads 0 failed across all suites. audio::jack_backend::tests::meas_ring_capacity_fits_stimulus_duration_and_tail_budget_at_every_rate (the renamed/extended test) is ok. The it_set_drive timing flakes recorded in earlier passes did not reproduce this run.

delta review (942c0e27..c71b6995)

One file, ac-rs/crates/ac-daemon/src/audio/jack_backend.rs (+43/-40): RING_CAPACITY (a fixed usize constant) is replaced by meas_ring_capacity(sample_rate: u32) -> usize, computed as ceil(2 * MAX_STIMULUS_DURATION_S * sample_rate), and start() now allocates the ring with meas_ring_capacity(self.sample_rate) instead of the constant.

Checked, not taken on the commit message's word:

• Ordering. self.sample_rate = client.sample_rate() as u32 (line 300) runs before HeapRb::<f32>::new(meas_ring_capacity(self.sample_rate)) (line 316), both inside the same start() call — the ring is sized from the rate JACK just reported, not a stale/default value. Read both lines directly.
• Formula. MAX_STIMULUS_DURATION_S is 60.0 (confirmed at handlers/mod.rs:486, same import used previously, not re-duplicated). 2 * 60.0 = 120, matching the doc comment's "twice that combined" (both duration and tail_s are independently capped at 60 s, so a request can combine up to 120 s — this matches the per-field validation in plot.rs already checked in an earlier pass). All rates in the test (44.1 k/48 k/96 k/192 k/384 k) multiply out to integers in this formula, so .ceil() is a no-op and there's no rounding-direction risk at any of them.
• No orphaned reference to the old constant. grep -rn "RING_CAPACITY\b" across ac-daemon/src turns up only FAKE_RING_CAPACITY (audio/fake/ring_mode.rs) and REF_RING_CAPACITY (jack_backend.rs:542, untouched by this delta) — both distinct, unrelated constants (fake backend synthesizes directly with no SPSC ring budget concern; ref-input ring is sized for transfer_stream's ~2.5 s capture, not the plot_ir duration+tail budget). Neither needed a matching change; scope is correctly narrow.
• 384 kHz claim verified, not trusted. ac-core/src/visualize/mtw/ladder.rs:482 does call layout(384_000) in its own test — the doc comment's claim that this rate is already exercised elsewhere in the tree is accurate.
• Test reachability. meas_ring_capacity_fits_stimulus_duration_and_tail_budget_at_every_rate asserts meas_ring_capacity(sr) >= (max_combined_s * sr) as usize for five rates including 384 k (the exact rate codex-qa's finding named). This is a genuine strengthening over the version it replaced, which only checked up to 192 kHz.

one gap, flagged by the developer's own commit message and independently confirmed

The commit message includes a self-reported "known weakness": the new test computes its own expected value with the same formula (meas_ring_capacity) the implementation uses, so cap >= n_total holds near-tautologically — it doesn't exercise start() at all, and wouldn't catch a future regression where start() stops calling meas_ring_capacity(self.sample_rate) (e.g., reverts to an inlined literal) while the function itself stays correct. I confirmed this is real: there is no test in this file, or elsewhere in ac-daemon, that calls JackEngine::start() against a real JACK client and inspects the resulting ring's actual capacity (ringbuf::traits::Observer::capacity(), already in scope via the existing use ringbuf::traits::Observer; at the top of mod tests) — the two tests that do call .start() against real JACK (jack_contiguous_drain_discards_nothing_and_keeps_up, jack_capture_multi_discards_live_audio_between_ticks, both #[ignore]d, requiring AC_TEST_CAPTURE_PORT) check drain/discard behavior, not ring capacity. This can't be closed without a real JACK server — CaptureRings also has no accessor exposing meas's capacity today (only occupied()), so closing it needs a small production addition, not just a test. Non-blocking: I read the actual wiring at start() (line 300/316 above) and confirmed it's correct at this tip; the gap is that nothing enforces it going forward, in an integration path this repo already accepts as #[ignore]d/rig-only for this exact backend.

suggested test addition (requires a small accessor first — flagging, not a blocking ask)

// Requires: CaptureRings exposes `pub(crate) fn meas_capacity(&self) -> usize`
// (e.g. `self.meas.as_ref().map(|c| c.capacity().get()).unwrap_or(0)`).
#[test]
#[ignore = "requires a running JACK server; set AC_TEST_CAPTURE_PORT"]
fn jack_start_sizes_ring_from_live_sample_rate() {
    let port = std::env::var("AC_TEST_CAPTURE_PORT")
        .unwrap_or_else(|_| "system:capture_1".to_string());
    let mut eng = JackEngine::new();
    eng.start(&[], Some(&port)).expect("JACK start");
    let sr = eng.sample_rate();
    assert_eq!(
        eng.rings.meas_capacity(),
        meas_ring_capacity(sr),
        "ring capacity at sr={sr} must track the live rate, not a fixed assumption"
    );
    eng.stop();
}

This is manual/rig-run-only (same as the two existing #[ignore]d tests in this file) — nothing in CI would notice if it broke, since this repo runs no pipeline on branches. Recording that plainly rather than implying coverage that isn't there.

points from my 942c0e27 review — status

1. Cancellation terminates stimulus/capture on all three backends — untouched by this delta, still holds.
2. Numeric budget ceilings tagged assumed, rig evidence outstanding — untouched, still open. requires-rig stays.
3. Non-blocking note from that pass (test comment in plot_ir.rs misdescribes the worker-reap mechanism) — not addressed, and not touched by this delta (which only edits jack_backend.rs). Still open, still non-blocking — flagging again so it isn't silently dropped.
4. Scope — this delta stays in scope: jack_backend.rs is in the PR's own file manifest from its opening description.
5. The codex-qa major finding against 942c0e27 (live-rate ring sizing) — addressed by this commit; verified independently above, not just by the commit message's claim.

spec coverage

criterion	provenance	covered	notes
Stopping plot_ir terminates stimulus and capture promptly on every supported backend	measured	✓	Unchanged by this delta.
Duration, tail, point density, step count, harmonic count, and window length are rejected before worker spawn when outside explicit finite budgets	assumed (architect, #427)	✓ (mechanism), ceiling values still open	This delta closes a real hole in the mechanism: the ring backing that budget now fits it at any live JACK rate, not just up to 192 kHz. Ceiling values remain rig-unverified, unchanged from every prior pass.
Rejected requests emit no audio and return an observable error	measured	✓	Unchanged by this delta.
Regression coverage includes cancellation during stimulus, cancellation during tail, each budget boundary, non-finite input, and integer-conversion overflow	measured	✓	Strengthened at the formula level (five rates including 384 kHz vs. four before); see the wiring-level gap noted above, which this criterion doesn't fully close for the reason given.

standards conformance

standards check: not applicable — scope-none (issue #427's own label, reconfirmed directly on the issue this pass: bug, ready-to-implement, agent:triage, scope-none). Checked the delta against docs/architecture/standards.md's document map directly (grep -n "jack_backend\|audio/" /home/mui/src/ac/docs/architecture/standards.md — no match for this file or path); this delta sizes an internal SPSC capture-ring buffer from the live sample rate, it does not touch a measurement formula, unit, or displayed value.

correctness issues

none found in the delta.

test coverage gaps

• The new/renamed formula test doesn't exercise start()'s actual wiring of meas_ring_capacity(self.sample_rate) — see the gap discussed above. Non-blocking: unfixable without a real JACK server in this environment, matches the pattern of two pre-existing #[ignore]d real-JACK tests already in this file, and the developer's own commit message flagged the same gap rather than concealing it. Suggested #[ignore]d addition given above.

scope issues

none.

verdict

approve
The delta correctly fixes the codex-qa-identified defect (fixed 192 kHz-sized ring silently failing above that rate) by deriving capacity from the live JACK rate at start(), verified independently by reading the actual ordering and formula rather than trusting the commit message. Full workspace gate (fmt, clippy -D warnings, test --workspace, check --features cpal-audio) is clean at tip c71b6995. One self-reported test-wiring gap is real but non-blocking (unclosable without real JACK hardware, matches existing #[ignore] precedent in this file). All open items from prior passes (assumed numeric ceilings, requires-rig, and the still-unaddressed non-blocking plot_ir.rs comment note) are unchanged by this push.

sent back to

no

rig verification required

Unchanged in substance from all prior passes: run plot_ir at the documented ceilings (60.0 s duration, 60.0 s tail, 32 harmonics, a 1,048,576-sample window, a 10,000-point sweep) and record peak memory, peak CPU, and stop-to-silence latency. This delta generalizes the ring-sizing fix to any live rate but doesn't change what needs measuring at the documented ceiling — requires-rig stays; only a human clears it. Separately, note the 384 kHz path this delta specifically targets is not reachable on the project's currently-documented dedicated rig hardware (FF400, which tops out at 192 kHz) — the fix is verified here at the formula level for that rate, but no rig run on existing hardware can exercise it; not requesting a new rig-verify-queue block for it on that basis.

label note

claude-approved predates c71b6995 (per the standing approval-voiding rule, any commit after an approval reverts the PR to needs-work until re-reviewed). This review re-validates the gate and the diff at the current tip with a fresh approve — re-applying claude-approved. in-review and requires-rig were already present and stay as-is.

[mkovero]

rig-2026-09-15-pr437-c71b6995 — rig record (re-run on the current tip)

Date (UTC): 2026-09-15 14:21–14:35 · Rig: pupu · Operator: Markus Kovero · Run by: Claude Code session
Why: the earlier records measured a5b7d76. c71b699 changed exactly what they measured: the JACK measurement ring is now sized from the live sample rate (meas_ring_capacity), where it was a fixed 120 s × 192 kHz. This re-runs the same requests on the current tip, so requires-rig can be judged against the code that would merge.

Build under test

rev c71b699 (dirty 0, compiled_this_run=yes: Compiling ac-core, ac-daemon, ac-cli in the build log), portable rustflags, staged /home/mui/ac-test/c71b69956b09-x86_64, sha256 verified on the rig. ac-daemon sha256 c9e5f7a0… differs from a5b7d76's 5a176b63…, so a stale build is ruled out. Not installed.

Pre-flight, wiring, clock

• JACK socket. jackd (started 00:35 EEST) was running, but its socket was gone again: 0 system ports, no /dev/shm entries, logind RemoveIPC after the last logout. I held a logind session (3 sessions for mui), then sudo systemctl restart jack-ac.service → 28 ports.
• FF400 baseline. Forced back with toggle writes (a restart resets the device while ALSA readback keeps baseline values).
• Preflight. All checks passed: 96000/256/-S, analog capture block 1-8.
• Wiring probe. probe-outputs.sh pupu --level -60 --outputs 1 --rev installed → IN2 tone −57.5 dBFS (baseline), IN1 −69.0 dBFS rms (mic alive). This was also the first real-hardware run of docs(rig): one runbook and scripts for testing binaries on a rig #441's fail-closed probe_remote.sh, exit 0.
• Wiring AN2 → IN2 unchanged. FF400 internal clock.

Emission consent

Operator, 2026-09-15, choosing "Rig re-run, then I clear" on the question naming the scope: loopback AN2→IN2 only, −40 dBFS, plot_ir at the ceilings plus soak, OK to emit this session. Ceiling enforced server-side: drive_max_dbfs -40.0 in the running daemon's config (isolated HOME /home/mui/ac-test/runs/437c/home, output_port system:playback_2, input_port system:capture_2). Daemon identity checked over ssh (pid 36413 → staged exe, HOME) and again over ZMQ status before the first request.

Runs — c71b699 vs a5b7d76 (same driver, same requests)

run	request (−40 dBFS, loopback)	c71b699	a5b7d76	peak RSS c71b / a5b7	CPU peak / mean c71b
R1c	duration 60, tail 0.5, 5 harm, win 4096	IR +65.32 s, report +65.34, done +65.35 s	done +64.2 s	1059.9 / 1059.8 MiB	100 % / 11.4 %
R3c	duration 2, tail 60	IR +64.40, report +64.42, done +64.43 s	done +64.5 s	419.6 / 383.7 MiB	133 % / 9.7 %
R7c	duration 60, tail 60, 32 harm, win 1,048,576	IR +129.34, report +132.17, done +133.60 s	done +134.9 s	1938.0 / 1904.2 MiB	100 % / 9.3 %
R8c	monitor_spectrum capture_2, 300 s, silent	stop reply stimulus silent	—	226.4 / 320.7 MiB	16.6 %

IN2 peak −37.1 dBFS on R1c/R7c, −36.9 on R3c. That is consistent with −40 nominal + 2.5 dB cable.

Result — PASS for the ceilings on c71b699 at 96 kHz

Every documented plot_ir ceiling, including the 120 s all-limits request, completes with an IR, a report and done on the current tip. The ring-sizing change broke none of the budget completions the earlier re-run established.

Readouts, not gates

• Analysis memory is unchanged in character. R1c and R7c peaks are within 0–2 % of a5b7d76's. R3c is +36 MiB (+9 %). Both builds analyse the same capture lengths. These are single 1 s pidstat samples, and code merged into this branch since a5b7d76 could also account for a few MiB, so I don't attribute the delta to the ring change, which shrinks the ring at 96 kHz. The all-limits peak is still 1.94 GiB, 25 % of pupu's 7.8 GiB.
• Monitor soak is flat and lower. RSS went 66 → 226 MiB by the first 5 s sample and stayed at 226 MiB for the whole 300 s, then fell to 124 MiB on stop. a5b7d76 climbed linearly (+0.366 MiB/s, the write head faulting in fresh ring pages) from 234 MiB to a 321 MiB plateau after 240 s. So the new build peaks ~95 MiB lower and shows no growth, consistent with a smaller ring. Why the growth phase is gone is not established here (ring touched at start vs. a different allocation on the monitor path). It's a readout, not a mechanism.

Confound

• One rate (96 kHz). The 384 kHz case that motivated meas_ring_capacity was not exercised on hardware; it is covered only by the unit test, whose own limitation is noted in the PR.
• pidstat at 1 s (runs) / 5 s (soak); a sub-second peak can read low.

Rig state left behind

Daemon stopped (no ac-daemon). JACK 96000/256/-S with 28 ports. FF400 at the forced, probe-verified baseline. Rig ~/.config/ac/config.json untouched (isolated HOME). A local systemd unit ac-pupu-hold still holds a logind session so the JACK socket survives. Stopping it lets RemoveIPC remove the socket on the next logout.

What should happen next

1. requires-rig: evidence for the ceilings now exists on the current tip. Clearing is the operator's gate.
2. After clearing: bin/review.sh --independent 437. The Codex gate skips requires-rig PRs by design.

[mkovero]

codex qa — PR #437 at c71b699

verdict: pass

gate

Claude QA workspace gate: inherited at current tip c71b69956b09129293034e9db35778634a3ba2d5
Codex targeted tests: not needed

All four issue #427 acceptance criteria are covered by the diff and changed tests. The explicitly assumed numeric ceilings are enforced before port resolution or worker spawn. Current-tip rig evidence confirms the combined plot_ir ceilings and measured resource/stop behavior at 96 kHz; the separating measurement for a live-rate sizing alternative is the same completion run at another JACK rate (384 kHz remains formula-tested because the available rig tops out at 192 kHz). No implementation defect follows from that hardware limit. GitHub reports no CI checks on the branch.

unaddressed open questions

none

scope

none

[mkovero]

Merged origin/main into this branch to clear the conflict in tests/it_protocol/out_of_range.rs (both sides added tests there). Behavioural collision, resolved by operator decision: this branch rejects duration: 0 for plot/plot_level/plot_ir before spawn, as ZMQ.md documents ("greater than 0"), while #428's merged test plot_duration_zero_fails_atomically_instead_of_archiving_partial_sweep sent plot with duration: 0.0 expecting ok: true. The > 0 rule stays. The test now uses duration: 0.001 and is renamed plot_tiny_duration_fails_atomically_instead_of_archiving_partial_sweep: max(0.001, 3.0 / freq) equals 3.0 / freq at every point of that sweep, so the partial-sweep failure it pins is unchanged. cargo fmt --check, cargo clippy -- -D warnings, cargo test --workspace pass at the merge commit. The prior approvals were at c71b699, so this tip needs fresh Claude QA and Codex.

[mkovero]

Re-review. Commit range reviewed: c71b69956b09129293034e9db35778634a3ba2d5..41b18bb090bff4b596fe16cf82a23bdcc0cc4357. Prior review (approve) was against c71b6995; that approval was voided the moment 41b18bb0 landed on top of it, per the standing rule.

what the delta actually is

41b18bb0 is Merge origin/main into issue-427-cancel-bound-plot-ir (parents: c71b6995, 39aebec5). Confirmed by diffing PR #437's own files between the two tips — every file in this PR's own manifest (jack_backend.rs, cpal_backend.rs, audio/mod.rs, handlers/admin.rs, handlers/mod.rs, ac-cli/src/commands/stop.rs, tests/it_protocol/plot_ir.rs) shows zero lines changed. The only PR-437-relevant content in this range is the merge's own conflict resolution in tests/it_protocol/out_of_range.rs; everything else (.agents/*, bin/*, ac-cli/calibrate.rs, calibrate/tau/*, sweep/harmonics.rs, ir_stats.rs, it_loopback_ir.rs, most of ZMQ.md) is main's own already-merged content (#428, #368, #369, #444, #446, agent-tooling fixes) arriving via catch-up merge, not new work on this branch.

the one substantive thing to check: the conflict resolution

Both branches added a test at the same spot in out_of_range.rs. Main's #428 test sent plot with duration: 0.0 expecting ok: true; this branch's own request-budget code rejects duration <= 0 before spawn (bounded_duration(..., zero_allowed: false, ...), handlers/audio/plot.rs:77), which ZMQ.md already documented ("greater than 0"). Operator resolved the collision by keeping the > 0 rule and moving main's test to duration: 0.001, renamed plot_tiny_duration_fails_atomically_instead_of_archiving_partial_sweep.

Verified the arithmetic claim independently, not just the commit message:

• bounded_duration(cmd, "duration", 1.0, false, "plot") at plot.rs:120 — zero_allowed=false → duration=0.001 clears the pre-spawn gate (matches the rule ZMQ.md documents).
• Sweep is 100–2000 Hz. Per-point capture floor is max(duration, 3.0/freq). At every frequency in that band, 3.0/freq ≥ 3.0/2000 = 0.0015 s > 0.001 s, so the floor is 3.0/freq at every point — the 0.0 vs 0.001 substitution changes nothing about which points fail.
• 3.0/freq × 48000 = 256 at freq = 562.5 Hz — the analyzer's 256-sample minimum, so points above 562.5 Hz fail and points at/below it succeed, matching the test's own assertions (completed > 0, completed < requested, message contains "256").
• Ran it directly rather than trusting the merge commit's report: cargo test -p ac-daemon --test it_protocol out_of_range::plot_tiny_duration_fails_atomically_instead_of_archiving_partial_sweep — passes, and it's a real assertion against a real daemon round-trip (not a fake/mock), so it can and does fail on the defect it names.

No trace of the old not_measured_no_loopback tau state left dangling except a documented backward-compat comment in ac-cli/src/commands/calibrate.rs:296 (still routes through the catch-all _ arm on the raw wire string) — not a defect.

mechanical gate, re-run at 41b18bb (workspace, not per-crate)

• cargo fmt --check — clean.
• cargo clippy --workspace --all-targets -- -D warnings — clean.
• cargo check --workspace --features cpal-audio — clean.
• cargo test --workspace -- --test-threads=4 — all green, no failures, no flakes this run (unlike the prior merge 83b8d01, which had one concurrency-flaky it_protocol test unrelated to plot code). Re-ran cargo test -p ac-daemon --test it_protocol in isolation too: 120/120 passed, including the renamed merge-resolution test.

points from my c71b6995 review — status

1. Cancellation terminates stimulus/capture on all three backends — untouched by this delta (0-line diff on the backend files). Still holds.
2. Numeric budget ceilings tagged assumed — untouched by this delta. Rig evidence: a human-cleared, codex-qa-confirmed rig run against this exact code (rig-2026-09-15-pr437-c71b6995) already exists and covers it; requires-rig was removed by the operator (2026-09-15 14:58) after that record posted, which is the only party allowed to clear it. Since none of the measured code changed in this delta, that clearance still applies to 41b18bb0.
3. Non-blocking note (test comment in plot_ir.rs misdescribes the worker-reap mechanism) — still not addressed, and this delta doesn't touch that file (0-line diff). Flagging again so it isn't silently dropped a second time.
4. Scope — the merge is well within scope: it's a documented catch-up with main, not developer scope creep, and it doesn't touch any file in the PR's own manifest.
5. codex-qa's ring-sizing finding — unaffected by this delta (already resolved at c71b6995, files untouched since).

spec coverage

criterion	provenance	covered	notes
Stopping plot_ir terminates stimulus and capture promptly on every supported backend	measured	✓	Unchanged by this delta.
Duration, tail, point density, step count, harmonic count, and window length are rejected before worker spawn when outside explicit finite budgets	assumed (architect, #427)	✓ (mechanism); ceiling values rig-verified this pass	Rig record rig-2026-09-15-pr437-c71b6995 measured all documented ceilings on hardware at the code now present in 41b18bb0 (files unchanged since); operator cleared requires-rig on that basis.
Rejected requests emit no audio and return an observable error	measured	✓	Unchanged by this delta.
Regression coverage includes cancellation during stimulus, cancellation during tail, each budget boundary, non-finite input, and integer-conversion overflow	measured	✓	Unchanged by this delta; the merge-resolution test is main's #428 coverage relocated, not new #427 coverage.

standards conformance

standards check: not applicable — issue #427 is still labelled scope-none (reconfirmed directly on the issue this pass), and this delta touches none of PR #437's own files, so there's nothing new to raise the label against. The delta does carry other PRs' output-format changes (#428's error frame fields, #369's tau_state values) into this branch, but those are main's already-merged, already-gated content under their own issues — not this PR's diff to answer for.

correctness issues

none found in the delta.

test coverage gaps

coverage is adequate for this delta. (Standing, non-blocking note: plot_ir.rs's test comment misdescribing the reap mechanism — item 3 above — remains open and untouched.)

scope issues

none — the merge touches no file outside main's own already-merged history; nothing in PR #437's own manifest changed.

verdict

approve
Mechanical gate is clean at 41b18bb0 (fmt, clippy -D warnings, cpal-audio check, full cargo test --workspace, no flakes this run), the only PR-relevant content in the delta (the out_of_range.rs merge conflict resolution) is independently verified correct and its test is reachable and passing, and every open item from the prior pass is either still valid (assumed ceilings, now rig-verified and human-cleared; the non-blocking plot_ir.rs comment note) or confirmed unaffected (all three backend files, handlers/mod.rs, stop.rs — zero lines touched).

sent back to

no

rig verification required

no — rig-2026-09-15-pr437-c71b6995 already measured all documented plot_ir ceilings against this exact code (unchanged since), and the operator cleared requires-rig on that record. This delta doesn't touch any measured file, so that clearance carries forward to 41b18bb0 unchanged. (Confound noted in that record stands: 384 kHz remains formula-tested only, since the dedicated rig's FF400 tops out at 192 kHz — not grounds for a new queue entry, per the prior pass's same conclusion.)

label note

Both claude-approved and codex-approved currently on the PR predate 41b18bb0 (last applied 09:14 and 15:06 respectively, before the 15:32 merge push) — stale per the approval-voiding rule. Re-applying claude-approved fresh against this tip. Not touching codex-approved (human/Codex-only); it will need its own re-run.